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

cli: omit ANSI colors when not a TTY or NO_COLOR is set #13

Merged
merged 1 commit into from
Jul 31, 2023

Conversation

wchargin
Copy link
Contributor

@wchargin wchargin commented Jul 8, 2023

  • I have read the Contributor Guide
  • I have read and agree to the Code of Conduct
  • I have added a description of my changes and why I'd like them included in the section below

The colors used by gsutil are unreadable on my light-background color
scheme, and gsutil does not respect either the NO_COLOR env var or
being piped to a non-TTY stream. This patch makes it respect both.

I looked for all matches of git grep -P '(?<!cc)\.paint', which
previously matched ls, main, setmeta, and stat, and now only
matches the internals of the new color module. If there are other ways
that ANSI codes are emitted, I may have missed them.

Addresses #11. Arguably fixes it via the environment variable; an
explicit command line option --no-color could still be added, which
this patch makes easy.

This uses the recently-stabilized IsTerminal trait, which bumps the
MSRV to 1.70.0.

wchargin-branch: no-color

The colors used by `gsutil` are unreadable on my light-background color
scheme, and `gsutil` does not respect either the [`NO_COLOR`] env var or
being piped to a non-TTY stream. This patch makes it respect both.

I looked for all matches of `git grep -P '(?<!cc)\.paint'`, which
previously matched `ls`, `main`, `setmeta`, and `stat`, and now only
matches the internals of the new `color` module. If there are other ways
that ANSI codes are emitted, I may have missed them.

Addresses EmbarkStudios#11. Arguably fixes it via the environment variable; an
explicit command line option `--no-color` could still be added, which
this patch makes easy.

This uses the recently-stabilized [`IsTerminal`] trait, which bumps the
MSRV to 1.70.0.

[`IsTerminal`]: https://doc.rust-lang.org/std/io/trait.IsTerminal.html
[`NO_COLOR`]: https://no-color.org

wchargin-branch: no-color
wchargin-source: e7b6232c54d34a1af61ebdd9a1e0a0eb31fe0324
@wchargin wchargin requested a review from Jake-Shadle as a code owner July 8, 2023 22:43
@Jake-Shadle Jake-Shadle merged commit 13fd33f into EmbarkStudios:main Jul 31, 2023
@wchargin wchargin deleted the wchargin-no-color branch August 1, 2023 17:59
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.

2 participants