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

Support FORCE_COLOR and NO_COLOR #3955

Closed
hugovk opened this issue Jun 1, 2024 · 11 comments · Fixed by #3979
Closed

Support FORCE_COLOR and NO_COLOR #3955

hugovk opened this issue Jun 1, 2024 · 11 comments · Fixed by #3979
Labels
configuration Settings and such enhancement New feature or improvement to existing functionality help wanted Contribution especially encouraged

Comments

@hugovk
Copy link

hugovk commented Jun 1, 2024

https://force-color.org and https://no-color.org are "community standards" supported by many tools, such as ruff, pip, pytest, rich, nox, tox.

It would be nice to set the FORCE_COLOR: 1 env var in GitHub Actions so I can have coloured output in the CI. It makes logs more readable, especially when there's a failure.

uv does have a --color CLI option, but more and more tools are using uv behind the scenes (such as https://github.com/hynek/build-and-inspect-python-package) so I don't have direct access to the uv invocation.

@charliermarsh
Copy link
Member

Yeah good call -- I think we might respect one of these... NO_COLOR, at least? Via https://docs.rs/anstream/latest/anstream/struct.AutoStream.html. I'll look into it.

@charliermarsh
Copy link
Member

Yeah we do support NO_COLOR, but not FORCE_COLOR=1.

@charliermarsh charliermarsh added enhancement New feature or improvement to existing functionality help wanted Contribution especially encouraged configuration Settings and such labels Jun 1, 2024
@samypr100
Copy link
Collaborator

samypr100 commented Jun 1, 2024

It does seem like anstream already supports CLICOLOR_FORCE which seems to do the same (in premise) as FORCE_COLOR, so maybe a doc update?

@charliermarsh
Copy link
Member

We might need to validate the precedence here too with the command-line arguments.

@hugovk
Copy link
Author

hugovk commented Jun 2, 2024

I already have FORCE_COLOR on the CI for other tools. I'd prefer if uv supported FORCE_COLOR, so I don't need to add an extra env var just for uv.

I forgot to mention it earlier, CPython 3.13 also supports FORCE_COLOR:

@charliermarsh
Copy link
Member

Yeah we should support it -- we also support it in Ruff for this reason.

@charliermarsh
Copy link
Member

Thanks for looking into it in anstream @samypr100, you rock.

@epage
Copy link

epage commented Jun 3, 2024

Should we add CLICOLOR_FORCE as well so that it's respected over the color CLI argument?

FYI anstream respects NO_COLOR and CLICOLOR / CLICOLOR_FORCE. Until recently, FORCE_COLOR has been very ad-hoc with people copying patterns of it and extending it in bespoke ways without any coordination. Looks like someone is now trying to coordinate that effort though I think I want to be cautious and see how well their variant takes off vs any other. I'm also concerned that both CLICOLOR and FORCE_COLOR exist without specifying their interaction.

see also my comments on rust-cli/anstyle#192

@charliermarsh
Copy link
Member

Thanks @epage. For some reason FORCE_COLOR has become very popular in the Python ecosystem -- you can see all the listed projects here: astral-sh/ruff#5499 (comment)

@epage
Copy link

epage commented Jun 3, 2024

That raises even more questions for me as none of them implement https://force-color.org/ (though most are likely "close enough").

So what approach is "right"? https://force-color.org/ is just a website that some person put up within the last 9 months. That doesn't make it definitive.

@epage
Copy link

epage commented Jun 4, 2024

I've raised the discrepancies in donatj/force-color.org#30

charliermarsh pushed a commit that referenced this issue Jun 4, 2024
## Summary

Closes #3955

Adds explicit support to `NO_COLOR` and `FORCE_COLOR` via
GlobalSettings.

The order, per specs is now `NO_COLOR` > `FORCE_COLOR` > `color`.

This PR is a backup plan pending rust-cli/anstyle#192.

## Test Plan

Tested all cases locally for now; I didn't see existing tests for
GlobalSettings parsing.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
configuration Settings and such enhancement New feature or improvement to existing functionality help wanted Contribution especially encouraged
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants