Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Colors for ocaml >= 4.03 #87

Open
Gbury opened this issue May 20, 2016 · 13 comments
Open

Colors for ocaml >= 4.03 #87

Gbury opened this issue May 20, 2016 · 13 comments

Comments

@Gbury
Copy link

Gbury commented May 20, 2016

Ocaml 4.03 comes with shiny colors for error and warnings (which considerably improve readability) however, it seems the heuristics used currently by default suppresses the colored output when invoked by ocamlbuild, most likely because of the log file: colored output prints fine on the terminal, but is quite ugly in the build log.

For those who rarely directly read the log, forcing colored output from the compiler is a solution, but it doesn't really feel right.

@gasche
Copy link
Member

gasche commented May 20, 2016

Storing color escape codes in the log should be fine as long as users use color-aware tools to read the log (eg. less -R). I just tried and -tag "color(always)" does the job. We could technically try to get ocamlbuild to use color output by default, by changing the way it invokes the compiler, but that sounds like a lot of arguably-ugly work for something that shouldn't be handled at this level anyway -- note that the current heuristic used by the compiler is not flexible enough to just enable an environment variable and be done with it, we should probably change that.

I think a decent solution is to keep expecting users to write true: color(always) in their _tags files, and ask upstream OCaml for a more environment-global way to set this on -- an OCAML_USE_COLOR variable or whatever. Could I let you send this feature request upstream? (Maybe with a patch?)

@Gbury
Copy link
Author

Gbury commented May 24, 2016

Sure, I'll try and make a PR as soon as possible

@Gbury
Copy link
Author

Gbury commented May 24, 2016

Seems like there is already support for it using the OCAMLPARAM env variable, is it enough ?

@gasche
Copy link
Member

gasche commented May 24, 2016

I don't think this is enough, OCAMLPARAM is fragile and it is not clear when it is the correct design to use it. I would support a PR to introduce a separate environment variable that is advertised to the user; but I don't know what others would think of it, and there is indeed a risk that the feature be rejected.

@dbuenzli
Copy link
Collaborator

dbuenzli commented May 24, 2016

Please really don't use OCAMLPARAM. Also frankly having to add one more thing in each of my _tags file to improve end-user usability is not something I'm very keen on.

We could technically try to get ocamlbuild to use color output by default, by changing the way it invokes the compiler, but that sounds like a lot of arguably-ugly work for something that shouldn't be handled at this level anyway

@gasche What do you mean by this ? Where should this be handled then ?

@gasche
Copy link
Member

gasche commented May 24, 2016

I find it weird that build system's implementation would need to be modified because a compiler suddenly gained the ability to use colors on certain cases.

The compiler is trying to guess whether colors should be used in a particular case, and the current heuristics fail to activate colors for the build-system use-case, which is to run the compiler in non-interactive mode. Is it the build-system job to explicitly point out that, while not calling the compiler interactively, it still wants colors? But then how many other tools would have to be modified in the same way? Or should the compiler heuristics be changed to use colors in this family of use-cases? But then how should they be distinguished from the cases where we do not want color? (Do we always want colors?).

My gut feeling in this case is that we should delegate the decision to the user: give them a way to communicate to the compiler that they want color across the build-system invocation. This is why I suggested an environment variable. I think that would be flexible enough for what @Gbury wants (basically he can insert this in a .bashrc or whatever and that amounts to making colors enabled by default), and be useful for other tools than ocamlbuild without requiring a cross-concern code change. (Not OCAMLPARAM, we agree this is iffy.)

You could alternatively argue that the build-system, when invoked interactively, does redirect the output to a file for internal logging reasons but ultimately shows the output to the user as if they had invoked the compiler directly. And that it should therefore communicate to the compiler the fact that the output is intended for interactive use. It would be nice if this was the path of least resistance: if the natural option for a build-system (or other tools) to invoke the compiler did propagate this information naturally.

I would agree with modifying ocamlbuild so that if behaves in this way, but I would like the change not to be too ad-hoc or heavy-handed. for example always passing -color always to the ocaml compiler seems ad-hoc; what about clang color support then, should I also know about it and arrange for it specifically? For such a change to be graceful, I may need a better-designed heuristic on the compiler side.

To summarize: in a perfect world, ocamlbuild and all other tools would do the right thing by default because the compiler heuristic just works. Maybe that cannot exist, and ocamlbuild and similar tools should be changed to propagate to their callees the intent to interactively show the result; but then I would like a way to do this that works as uniformly as possible across the called tools that do have a specific user-facing-output mode, instead of having to single-case each tool (here the OCaml compiler). I am not sure whether this is possible today (OCaml and Clang's heuristics are good enough for that) or whether extra compiler-side support is needed.

@dbuenzli
Copy link
Collaborator

Le mardi, 24 mai 2016 à 16:40, Gabriel Scherer a écrit :

I find it weird that build system's implementation would need to be modified because a compiler suddenly gained the ability to use colors on certain cases.

Well if the build system has internal rules which are responsible to invoke the actual compilers that gained that ability, I personally don't find this weird.

The compiler is trying to guess whether colors should be used in a particular case, and the current heuristics fail to activate colors for the build-system use-case, which is to run the compiler in non-interactive mode.

This is the way any tool that produces colored output works.

Is it the build-system job to explicitly point out that, while not calling the compiler interactively, it still wants colors?
Yes. The build system is in charge of running the programs and redirect the feedback of the program to the end-user in the best way. More precisely it is not the build-system job but the job of the build system rules. The build system, however, should be responsible to provide the ability to the rules to check whether colored output should be enabled or not.

To summarize: in a perfect world, ocamlbuild and all other tools would do the right thing by default because the compiler heuristic just works.
AFAIK the heuristic is correct at the moment (i.e. no ansi escapes if stdout/stderr is not a tty). You can't have magic.

If you want a good end-user experience this is what should be done in my opinion. Add an option —color={auto,always,never} to ocamlbuild itself that eventually sets a global boolean color parameter and defaults to auto along with an OCAMLBUILD_COLOR that allows to override the default. In the auto case ocamlbuild detects whether its stdout/sdterr is a tty and sets the color parameter to true if that is the case and false otherwise. This color parameter must be propagated in the build rules for the tools that support colored output control.

Best,

Daniel

@gasche
Copy link
Member

gasche commented May 24, 2016

Thanks for the precise recommendation: it sounds reasonable and I can follow it. If you think that an OCAMLBUILD_COLOR environment variable is a good design for ocamlbuild, I suppose that you also agree with my suggestion to ask for an OCAML_COLOR environment variable to the upstream compiler, independently of what we do on the ocamlbuild side?

@dbuenzli
Copy link
Collaborator

If you think that an OCAMLBUILD_COLOR environment variable is a good design for ocamlbuild, I suppose that you also agree with my suggestion to ask for an OCAML_COLOR environment variable to the upstream compiler, independently of what we do on the ocamlbuild side?

Yes. Note, the TOOL_COLOR variables are only here to change the default color value if nothing is specified on the cli, i.e. it overrides the default value (which should be auto).

@gasche
Copy link
Member

gasche commented May 24, 2016

Yep, the correct cascading order is "env < cli < file-local settings".

@Gbury
Copy link
Author

Gbury commented Jun 8, 2016

So, I'd like to try and do a PR on the ocaml repo to add support for the OCAML_COLOR env variable. However, since OCAMLPARAM is also currently used, what should be the priority between the two, or would it be better to drop the color support in OCAMLPARAM ?

@gasche
Copy link
Member

gasche commented Jun 8, 2016

I would let the code speak. I think the most natural place to test OCAML_COLOR is the should_enable_color test in utils/misc.ml. If you do this, then it will be called exactly when the color setting is auto, which means that OCAMLPARAM will be able to either supersede it (by using a non-auto setting) or defer to it (by using auto).

@hannesm
Copy link
Member

hannesm commented Mar 11, 2017

I opened ocaml/ocaml#1098 to solve this issue (hope I understood the discussion in here correctly), and hope that it will be considered for the 4.05 release (fingers crossed).

hannesm added a commit to hannesm/ocaml that referenced this issue Mar 15, 2017
…colors

Since 4.03, OCaml supports coloring its messages to standard output and standard
error, depending on the "-color" argument ({always,never,auto}).  This commit
adds support for the environment variable "OCAML_COLOR" (which value can as well
be {always,never,auto}).

The command line argument "-color" takes precedence, "OCAML_COLOR" is only
taken into consideration if no "-color" is provided.

The motivation for this is that the user should have control over coloring
OCaml's output messages.  OCamlbuild, a widely used build tool executes OCaml
not under a tty (and OCaml does not colorize errors and warnings), which lead
various packages use `color(always)` in their `_tags` files, which breaks with
other (non-interactive) programs (i.e.  editor helpers).

Further discussion was done at ocaml/ocamlbuild#87 and
ocaml#1098.
gasche pushed a commit to ocaml/ocaml that referenced this issue Mar 15, 2017
…colors

Since 4.03, OCaml supports coloring its messages to standard output and standard
error, depending on the "-color" argument ({always,never,auto}).  This commit
adds support for the environment variable "OCAML_COLOR" (which value can as well
be {always,never,auto}).

The command line argument "-color" takes precedence, "OCAML_COLOR" is only
taken into consideration if no "-color" is provided.

The motivation for this is that the user should have control over coloring
OCaml's output messages.  OCamlbuild, a widely used build tool executes OCaml
not under a tty (and OCaml does not colorize errors and warnings), which lead
various packages use `color(always)` in their `_tags` files, which breaks with
other (non-interactive) programs (i.e.  editor helpers).

Further discussion was done at ocaml/ocamlbuild#87 and
#1098.
camlspotter pushed a commit to camlspotter/ocaml that referenced this issue Oct 17, 2017
…colors

Since 4.03, OCaml supports coloring its messages to standard output and standard
error, depending on the "-color" argument ({always,never,auto}).  This commit
adds support for the environment variable "OCAML_COLOR" (which value can as well
be {always,never,auto}).

The command line argument "-color" takes precedence, "OCAML_COLOR" is only
taken into consideration if no "-color" is provided.

The motivation for this is that the user should have control over coloring
OCaml's output messages.  OCamlbuild, a widely used build tool executes OCaml
not under a tty (and OCaml does not colorize errors and warnings), which lead
various packages use `color(always)` in their `_tags` files, which breaks with
other (non-interactive) programs (i.e.  editor helpers).

Further discussion was done at ocaml/ocamlbuild#87 and
ocaml#1098.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants