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

https://no-color.org/ support #136

Closed
eitah opened this issue May 6, 2021 · 3 comments · Fixed by #137
Closed

https://no-color.org/ support #136

eitah opened this issue May 6, 2021 · 3 comments · Fixed by #137

Comments

@eitah
Copy link

eitah commented May 6, 2021

We're consumers of concourse-ci which uses this package to write to the CLI. We have an open ticket to their library to implement support for the NO_COLOR feature flag but realize they're likely dependent on changes to this lib to make that happen.

Our use case is we wish to write output from concourse-ci to a text file but the inline color codes make it tricky

More on the no-color.org movement: https://no-color.org/
The concourse feature request: https://github.com/concourse/rfcs/discussions/104
Concourse-ci.org: https://concourse-ci.org/ (unsurprisingly)

If you're short on time but want to support the feature, perhaps this is a beginner friendly ticket that I or a peer from my company could PR?

Thanks muchly!
Eli

@eitah
Copy link
Author

eitah commented May 6, 2021

It looks like per your readme the recommended approach to suppress colors is to call isatty to get color.NoColor implemented. Fly (concourse CLI) already uses no-color in tests, so perhaps it is on their app to enable globally? Forgive the pointless ticket if so and if no-color.org single environment variable approach is too heavy-handed then.

fatih added a commit that referenced this issue May 13, 2021
This PR adds support for the enviroment variable `NO_COLOR`. If set
(regardless of its value), the `colors` package disables color output.
For more information about this environment variable please checkout
this website: https://no-color.org

closes: #136
@fatih
Copy link
Owner

fatih commented May 13, 2021

Hi @eitah

I initially was against adding this environment variable (see: #92). But, now that I think, many people import this library, and people are using this without the power to change the code. This is especially true if you are using a CLI that uses fatih/color.

Hence, I reverted my decision and added support for it. Check out this PR: #137

@eitah
Copy link
Author

eitah commented May 23, 2021

Thanks! This is a great change and will be very useful for us in the future. I really appreciate the thought and care you put into this @fatih.

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 a pull request may close this issue.

2 participants