Add terminal.unix to re-expose size functions

[art-w]

In #38, I introduced a breaking change by removing the terminal size functions as they were dependent on unix and were incompatible with mirage usage. But I wrongly assumed that these functions were only used by progress, while there are in fact users who depended on them to measure the terminal dimensions (see #42). This new PR introduces terminal.unix to re-expose the size functions.

@dinosaure or @msprotz > Would you prefer if terminal.unix was self-contained? eg:

include Terminal
module Size = struct ... end

In which case, could we rename/swap the two libraries, eg terminal => terminal.ansi and terminal.unix => terminal, to fully hide the breaking change from existing users?

[msprotz]

I think it's a plus if it's self-contained as it makes the build faster, doesn't it? And yes not having any breakage is even more optimal. Thanks!

[art-w]

Yeah that kind of makes sense with the existing lib structure as progress depends on unix, while progress.engine does not. I've added a commit with the renaming such that terminal depends on unix and exposes the Size module (= backward compatibility), while terminal.ansi is the new unix-free lib usable for mirage. It's not 100% perfect so let me know if we can squash or if the renaming is not worth the effort :)

[art-w]

This is a bit ugly, but we have a diamond dependency:

terminal.ansi <------- terminal
 ^                       ^
 |                       |
progress.engine <----- progress

and progress needs to see that terminal and progress.engine are using the same color/style types.

[dinosaure]

I'm not sure to really understand the re_export field. That means that if unix is found as a dependency, we integrate terminate_ansi?

[art-w]

I don't understand re_export either and copied that trick from progress/dune, but it doesn't work without it as e.g. Terminal.Color is now a module alias to Terminal_ansi.Color and without re_export the Terminal_ansi would be missing and cause compilation error (so a user would need to always depend on both terminal terminal.ansi rather than just terminal)

(The re_export doesn't have anything to do with unix being available, a mirageos unikernel would depend on terminal.ansi and ignore the unix-dependent terminal lib, just like it would need to depend on progress.engine and wouldn't be able to use the unixy progress lib)

[dinosaure]

I'm not sure I understand this PR. I think I understand that you're trying to ‘revert’ #38 by re-exporting the same API but with a rather obscure trick to maintain compatibility with Mirage.

From my point of view, it might be ‘simpler’ to export a new Terminal_unix module available via the terminal.unix package and explain to users to use this new module to get system information if they wish.

[art-w]

No problemo, there are two commits in the PR :) The first commit adds terminal.unix with the unix dependency and size functions... but users will need to s/Terminal.Size/Terminal_unix and update their dependencies.

I'm happy to remove the second commit which attempts to restore backward compatibility by copying the design of the progress library:

• progress is unix dependent, while progress.engine is not and is mirage-friendly
• terminal is unix dependent, while terminal.ansi is not

I don't have a strong preference either way, but I think the original library organization was attempting for unix users to be the happy path (just depend and use progress, there is no progress.unix for the unix integration)

[dinosaure]

Ok, I see the trick now. It seems indeed the painless solution for @msprotz. I will merge an cut a release. Thanks for your time - I will probably extends your PR with some comments to explain the Mirage path.