8000 Support `CLICOLOR` and `CLICOLOR_FORCE` to manage ANSI colors by MisterDA · Pull Request #6340 · ocaml/dune · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Support CLICOLOR and CLICOLOR_FORCE to manage ANSI colors #6340

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

Merged
merged 2 commits into from
Oct 28, 2022

Conversation

MisterDA
Copy link
Contributor
@MisterDA MisterDA commented Oct 27, 2022

The patch follows the spec at https://bixense.com/clicolors/ that has been implemented by multiple build systems and compilers already.

When Dune is not run in a tty (such as a CI system or a script), it will strip ANSI colors from the output of targets (such a tests runs). This is often undesirable.

Dune will now recognize the following environment variables and act according to the spec:

  • CLICOLOR != 0
    ANSI colors are supported and should be used when the program isn’t piped.
  • CLICOLOR == 0
    Don’t output ANSI color escape codes.
  • CLICOLOR_FORCE != 0
    ANSI colors should be enabled no matter what.

The behaviour regarding TERM=dumb is also retained.

This allows toggling from the global state the output of colors.

Fixes #6323.

The patch follows the spec at <https://bixense.com/clicolors/> that
has been implemented by multiple build systems and compilers already.

When Dune is not run in a tty (such as a CI system or a script), it
will strip ANSI colors from the output of targets (such a tests
runs). This is often undesirable.

Dune will now recognize the following environment variables and act
according to the spec:

- CLICOLOR != 0
  ANSI colors are supported and should be used when the program isn’t
  piped.
- CLICOLOR == 0
  Don’t output ANSI color escape codes.
- CLICOLOR_FORCE != 0
  ANSI colors should be enabled no matter what.

The behaviour regarding TERM=dumb is also retained.

This allows toggling from the global state the output of colors.

Signed-off-by: Antonin Décimo <antonin@tarides.com>
Signed-off-by: Antonin Décimo <antonin@tarides.com>
@Alizter Alizter requested a review from emillon October 27, 2022 13:19
@rgrinberg rgrinberg added this to the 3.6.0 milestone Oct 27, 2022
@rgrinberg
Copy link
Member

LGTM. If this is some sort of a pseudo standard, does it make sense to extract a small lib like we did with xdg?

Can you comment on these issues:
#145
#2339

It would be good to get this colors stuff once and for all.

@MisterDA
Copy link
Contributor Author

If this is some sort of a pseudo standard, does it make sense to extract a small lib like we did with xdg?

I think this is short enough that we don't really need to extract it.

@rgrinberg
Copy link
Member

Okay. Do you think we still need a --color option?

@MisterDA
Copy link
Contributor Author

Okay. Do you think we still need a --color option?

It'd duplicate the functionality. If we support CLICOLOR and CLICOLOR_FORCE we don't need it, but it could be useful to override settings locally? or for discoverability. I don't think it can hurt anyway if the precedence of all these options is clear. I've also answered in #2339.

I'm not familiar enough with Dune to thread a --color argument from its definition point to this ansi module, though.

@rgrinberg rgrinberg merged commit 6de6d75 into ocaml:main Oct 28, 2022
@MisterDA MisterDA deleted the color-env-vars branch November 2, 2022 13:17
emillon added a commit to emillon/opam-repository that referenced this pull request Nov 14, 2022
…ne-site, dune-rpc, dune-rpc-lwt, dune-private-libs, dune-glob, dune-configurator, dune-build-info, dune-action-plugin and chrome-trace (3.6.0)

CHANGES:

- Forbid multiple instances of dune running concurrently in the same workspace.
  (ocaml/dune#6360, fixes ocaml/dune#236, @rgrinberg)

- Allow promoting into source directories specified by `subdir` (ocaml/dune#6404, fixes
  ocaml/dune#3502, @rgrinberg)

- Make dune describe workspace return the correct root path
  (ocaml/dune#6380, fixes ocaml/dune#6379, @esope)

- Introduce a `$ dune ocaml top-module` subcommand to load modules directly
  without sealing them behind the signature. (ocaml/dune#5940, @rgrinberg)

- [ctypes] do not mangle user written names in the ctypes stanza (ocaml/dune#6374, fixes
  ocaml/dune#5561, @rgrinberg)

- Support `CLICOLOR` and `CLICOLOR_FORCE` to enable/disable/force ANSI
  colors. (ocaml/dune#6340, fixes ocaml/dune#6323, @MisterDA).

- Forbid private libraries with `(package ..)` set from depending on private
  libraries that don't belong to a package (ocaml/dune#6385, fixes ocaml/dune#6153, @rgrinberg)

- Allow `Byte_complete` binaries to be installable (ocaml/dune#4873, @AltGr, @rgrinberg)

- Revive `$ dune external-lib-deps` under `$ dune describe external-lib-deps`.
  (ocaml/dune#6045, @moyodiallo)

- Fix running inline tests in bytecode mode (ocaml/dune#5622, fixes ocaml/dune#5515, @dariusf)

- [ctypes] always re-run `pkg-config` because we aren't tracking its external
  dependencies (ocaml/dune#6052, @rgrinberg)

- [ctypes] remove dependency on configurator in the generated rules (ocaml/dune#6052,
  @rgrinberg)

- Build progress status now shows number of failed jobs (ocaml/dune#6242, @Alizter)

- Allow absolute build directories to find public executables. For example,
  those specified with `(deps %{bin:...})` (ocaml/dune#6326, @anmonteiro)

- Create a fake socket file `_build/.rpc/dune` on windows to allow rpc clients
  to connect using the build directory. (ocaml/dune#6329, @rgrinberg)

- Prevent crash if absolute paths are used in the install stanza and in
  recursive globs. These cases now result in a user error. (ocaml/dune#6331, @gridbugs)

- Add `(glob_files <glob>)` and `(glob_files_rec <glob>)` terms to the `files`
  field of the `install` stanza (ocaml/dune#6250, closes ocaml/dune#6018, @gridbugs)

- Allow `:standard` in the `(modules)` field of the `coq.pp` stanza (ocaml/dune#6229,
  fixes ocaml/dune#2414, @Alizter)

- Fix passing of flags to dune coq top (ocaml/dune#6369, fixes ocaml/dune#6366, @Alizter)

- Extend the promotion CLI to a `dune promotion` group: `dune promote` is moved
  to `dune promotion apply` (the former still works) and the new `dune promotion
  diff` command can be used to just display the promotion without applying it.
  (ocaml/dune#6160, fixes ocaml/dune#5368, @emillon)
MisterDA added a commit to MisterDA/dune that referenced this pull request Nov 17, 2022
We want to enable the status line if and only if stderr is a tty. ANSI
color usage was tweaked in 3.6.0 with ocaml#6340 to allow users to force
colors. As status line activation actually depended on whether ANSI
colors were enabled or not, this change had the side effect to always
enable the status line even when ANSI colors were requested for
something that's not a tty.

Fix this by exposing a simple lazy wrapper of Unix.isatty to use
instead of Ansi_color.stderr_supports_color.
MisterDA added a commit to MisterDA/dune that referenced this pull request Nov 17, 2022
We want to enable the status line if and only if stderr is a tty. ANSI
color usage was tweaked in 3.6.0 with ocaml#6340 to allow users to force
colors. As status line activation actually depended on whether ANSI
colors were enabled or not, this change had the side effect to always
enable the status line even when ANSI colors were requested for
something that's not a tty.

Fix this by exposing a simple lazy wrapper of Unix.isatty to use
instead of Ansi_color.stderr_supports_color.

Signed-off-by: Antonin Décimo <antonin@tarides.com>
MisterDA added a commit to MisterDA/dune that referenced this pull request Nov 17, 2022
We want to enable the status line if and only if stderr is a tty. ANSI
color usage was tweaked in 3.6.0 with ocaml#6340 to allow users to force
colors. As status line activation actually depended on whether ANSI
colors were enabled or not, this change had the side effect to always
enable the status line even when ANSI colors were requested for
something that's not a tty.

Fix this by exposing a simple lazy wrapper of Unix.isatty to use
instead of Ansi_color.stderr_supports_color.

Signed-off-by: Antonin Décimo <antonin@tarides.com>
rgrinberg pushed a commit that referenced this pull request Nov 17, 2022
We want to enable the status line if and only if stderr is a tty. ANSI
color usage was tweaked in 3.6.0 with #6340 to allow users to force
colors. As status line activation actually depended on whether ANSI
colors were enabled or not, this change had the side effect to always
enable the status line even when ANSI colors were requested for
something that's not a tty.

Fix this by exposing a simple lazy wrapper of Unix.isatty to use
instead of Ansi_color.stderr_supports_color.

Signed-off-by: Antonin Décimo <antonin@tarides.com>
emillon pushed a commit to emillon/dune that referenced this pull request Nov 24, 2022
We want to enable the status line if and only if stderr is a tty. ANSI
color usage was tweaked in 3.6.0 with ocaml#6340 to allow users to force
colors. As status line activation actually depended on whether ANSI
colors were enabled or not, this change had the side effect to always
enable the status line even when ANSI colors were requested for
something that's not a tty.

Fix this by exposing a simple lazy wrapper of Unix.isatty to use
instead of Ansi_color.stderr_supports_color.

Signed-off-by: Antonin Décimo <antonin@tarides.com>
emillon added a commit that referenced this pull request Nov 24, 2022
We want to enable the status line if and only if stderr is a tty. ANSI
color usage was tweaked in 3.6.0 with #6340 to allow users to force
colors. As status line activation actually depended on whether ANSI
colors were enabled or not, this change had the side effect to always
enable the status line even when ANSI colors were requested for
something that's not a tty.

Fix this by exposing a simple lazy wrapper of Unix.isatty to use
instead of Ansi_color.stderr_supports_color.

Signed-off-by: Antonin Décimo <antonin@tarides.com>

Signed-off-by: Antonin Décimo <antonin@tarides.com>
Co-authored-by: Antonin Décimo <antonin@tarides.com>
@emillon emillon mentioned this pull request Nov 30, 2022
emillon added a commit to emillon/dune that referenced this pull request Nov 30, 2022
Since ocaml#6340 we're considering `CLICOLOR_FORCE` to determine whether
stderr supports color. However this changes behavior observable from the
test suite, so `CLICOLOR_FORCE=1 make test` fails (this is tracked as
ocaml#6607).

The issue is that ocaml/setup-ocaml#631, this variable is set in the CI
environment. So we disable it until the situation is fixed (and this
variable does not have observable changes anymore).

Signed-off-by: Etienne Millon <me@emillon.org>
emillon added a commit to emillon/dune that referenced this pull request Nov 30, 2022
Since ocaml#6340 we're considering `CLICOLOR_FORCE` to determine whether
stderr supports color. However this changes behavior observable from the
test suite, so `CLICOLOR_FORCE=1 make test` fails (this is tracked as
ocaml#6607).

The issue is that ocaml/setup-ocaml#631, this variable is set in the CI
environment. So we disable it until the situation is fixed (and this
variable does not have observable changes anymore).

Signed-off-by: Etienne Millon <me@emillon.org>
emillon added a commit to emillon/dune that referenced this pull request Nov 30, 2022
Since ocaml#6340 we're considering `CLICOLOR_FORCE` to determine whether
stderr supports color. However this changes behavior observable from the
test suite, so `CLICOLOR_FORCE=1 make test` fails (this is tracked as
ocaml#6607).

The issue is that ocaml/setup-ocaml#631, this variable is set in the CI
environment. So we disable it until the situation is fixed (and this
variable does not have observable changes anymore).

Signed-off-by: Etienne Millon <me@emillon.org>
emillon added a commit to emillon/dune that referenced this pull request Nov 30, 2022
Since ocaml#6340 we're considering `CLICOLOR_FORCE` to determine whether
stderr supports color. However this changes behavior observable from the
test suite, so `CLICOLOR_FORCE=1 make test` fails (this is tracked as
ocaml#6607).

The issue is that ocaml/setup-ocaml#631, this variable is set in the CI
environment. So we disable it until the situation is fixed (and this
variable does not have observable changes anymore).

Signed-off-by: Etienne Millon <me@emillon.org>
emillon added a commit to emillon/dune that referenced this pull request Nov 30, 2022
Since ocaml#6340 we're considering `CLICOLOR_FORCE` to determine whether
stderr supports color. However this changes behavior observable from the
test suite, so `CLICOLOR_FORCE=1 make test` fails (this is tracked as
ocaml#6607).

The issue is that ocaml/setup-ocaml#631, this variable is set in the CI
environment. So we disable it until the situation is fixed (and this
variable does not have observable changes anymore).

Signed-off-by: Etienne Millon <me@emillon.org>
emillon added a commit that referenced this pull request Nov 30, 2022
< 8000 div class="js-details-container Details js-socket-channel js-updatable-content" data-channel="eyJjIjoicmVwbzo3MjM2ODA1MDpjb21taXQ6YTA5M2JkMDE4M2Q3YTU1Njg1YWNmNzg5OWRhOTkxMGRhOTlkNTkzNCIsInQiOjE3NTEwNDc2MDh9--b3b1340a351c7259ce857cdc9e8f6012dc2f83f11bea3a0aa2b49a3fa63cc826" data-url="/ocaml/dune/commit/a093bd0183d7a55685acf7899da9910da99d5934/show_partial?partial=commit%2Fcondensed_details">
* Set CLICOLOR_FORCE=0

Since #6340 we're considering `CLICOLOR_FORCE` to determine whether
stderr supports color. However this changes behavior observable from the
test suite, so `CLICOLOR_FORCE=1 make test` fails (this is tracked as
#6607).

The issue is that ocaml/setup-ocaml#631, this variable is set in the CI
environment. So we disable it until the situation is fixed (and this
variable does not have observable changes anymore).

Signed-off-by: Etienne Millon <me@emillon.org>
moyodiallo pushed a commit to moyodiallo/dune that referenced this pull request Dec 2, 2022
* Set CLICOLOR_FORCE=0

Since ocaml#6340 we're considering `CLICOLOR_FORCE` to determine whether
stderr supports color. However this changes behavior observable from the
test suite, so `CLICOLOR_FORCE=1 make test` fails (this is tracked as
ocaml#6607).

The issue is that ocaml/setup-ocaml#631, this variable is set in the CI
environment. So we disable it until the situation is fixed (and this
variable does not have observable changes anymore).

Signed-off-by: Etienne Millon <me@emillon.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Retain ANSI escape sequences in buffered builds without a tty
2 participants
0