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

Enable colours when FORCE_COLOR env var is set #10909

Closed
1 task done
hugovk opened this issue Feb 14, 2022 · 20 comments
Closed
1 task done

Enable colours when FORCE_COLOR env var is set #10909

hugovk opened this issue Feb 14, 2022 · 20 comments
Labels
project: vendored dependency Related to a vendored dependency type: feature request Request for a new feature

Comments

@hugovk
Copy link
Contributor

hugovk commented Feb 14, 2022

What's the problem this feature will solve?

pip 22 now uses Rich for installation progress bars and some error handling, which gives us pretty colours!

image

Colour can help diagnose problems, and a common place for pip and the need to diagnose problems is on a CI.

However, because the GitHub Actions isn't a tty, output of tools which autodetect the terminal output is usually monochrome. That's why many tools and CI setups use the FORCE_COLOR environment variable, and/or command-line options to turn it back on.

The pip CI included (#10830):

env:
# The "FORCE_COLOR" variable, when set to 1,
# tells Nox to colorize itself.
FORCE_COLOR: "1"

However, pip itself (and Rich) don't use FORCE_COLOR, meaning we get monochrome output:

image

Compare with nox:

image

https://github.com/pypa/pip/runs/5177210581?check_suite_focus=true

And pytest:

image

https://github.com/pytest-dev/pytest/runs/5185781927?check_suite_focus=true

Describe the solution you'd like

Enable colour when FORCE_COLOR is set.

For reference:

Alternative Solutions

Get Rich to support FORCE_COLOR directly. This was considered in 2020 but rejected in Textualize/rich#343 to leave it up to the developer to support it.

But Will said "I may yet relent on this. I try to be pragmatic!" so it may still be a better option :)

Additional context

I'm not specifically requesting it here, but could be worth considering also disabling when NO_COLOR is set: https://no-color.org/

Similarly not requesting it here, but for completeness I'll mention pytest also has this command-line option:

  --color=color         color terminal output (yes/no/auto).

And nox:

  --nocolor, --no-color
                        Disable all color output.
  --forcecolor, --force-color
                        Force color output, even if stdout is not an interactive terminal.

Whereas pip only has the negative:

  --no-color                  Suppress colored output.

Code of Conduct

@hugovk hugovk added S: needs triage Issues/PRs that need to be triaged type: feature request Request for a new feature labels Feb 14, 2022
@hugovk hugovk changed the title Enable colours when FORCE_COLOR Enable colours when FORCE_COLOR env var is set Feb 14, 2022
@potiuk
Copy link
Contributor

potiuk commented Feb 14, 2022

Yeah. That would be indeed very useful. Colors in CI are great for visual communication (as otherwise you end up with big blob of text) - especially that often in Ci you enable pretty verbose output to be able to diagnose problems. Nany tools allows forcing colors indeed so it woud be useful to have it in 'pip'. Happy to help and create a PR for that one if preferred way to do it is agreed to by the maintainers..

@DiddiLeija DiddiLeija added the C: automation Automated checks, CI etc label Feb 14, 2022
@pradyunsg pradyunsg added project: vendored dependency Related to a vendored dependency and removed C: automation Automated checks, CI etc S: needs triage Issues/PRs that need to be triaged labels Feb 14, 2022
@hugovk
Copy link
Contributor Author

hugovk commented Mar 12, 2022

Let's also ping @willmcgugan from Rich: do you think it would be better for Rich itself to use FORCE_COLOR?

I think it would: better to do things upstream and it would mean other users of Rich would also benefit.

Please let me know if I should instead comment in Textualize/rich#343 or open a new Rich issue.

@willmcgugan
Copy link

I don't want to add FORCE_COLOR to Rich because on some apps FORCE_COLOR=0 would mean "force color off", but on other apps FORCE_COLOR set to anything means "force color on", i.e. the exact opposite.

Since there is no sane standard at the moment, I'd prefer to leave it up apps to decide how to do it. Fortunately it is trivial to implement it how you wish.

@potiuk
Copy link
Contributor

potiuk commented Mar 12, 2022

Agree with @willmcgugan :). Also not everyone uses rich (though I think it gets close to becoming de-facto standard in Pyhon world) so setting the standard here by rich brings no big value as just setting "FORCE_COLOR" for the whole CI would not cut it.

@hugovk
Copy link
Contributor Author

hugovk commented Mar 13, 2022

Okay, thanks Will!


For pip:

I checked the top 5,000 PyPI packages (with help from this and that):

Package Use PyPI downloads Feb 2022
pytest NO_COLOR any value -> off
FORCE_COLOR any value -> on
35,247,938
colorlog FORCE_COLOR any value -> on
NO_COLOR any value -> off
25,798,377
build NO_COLOR any value -> off
FORCE_COLOR any value -> on
1,301,248
nox FORCE_COLOR any value -> on
NO_COLOR any value -> off
923,535
molecule FORCE_COLOR truthy -> on
FORCE_COLOR falsy -> off
NO_COLOR any value -> off
267,105
Js2Py FORCE_COLOR any value -> on 199,216
enrich FORCE_COLOR truthy -> on
FORCE_COLOR falsy -> off
NO_COLOR any value -> off
403,707
colored FORCE_COLOR=1-3 -> on
FORCE_COLOR=0 -> off
NO_COLOR any value -> off
162,316
Total 64,303,442

The top four all just check for any value of the env var (difference in the order of precedence).

Those four account for over 98% of the downloads of these eight.

So how about pip (55%) follows the pytest pattern?

@ssbarnea
Copy link
Contributor

I have some good news, apparently colorama also approved adoption of a sister change tartley/colorama#230 (comment) and will likely be merged soon.

Based on the stats I seen, I think we have a clear winner. I know that two vars is less ideal but having this is still far better than one variable for each command line tool!. At least people recognized the pattern and moved on.

To be honest any generic environment variable is much better than MY_TOOL_NAME_USE_COLORS as that one that not scale at all. I see extremely unlikely that someone would want to enable colors only for one tool while not enabling them for other. Even if they want they can always override with FOO=bar cmd args ....

Funny that I see myself explaining the same thins I already did two years ago on rich, while the proposal was refused. I do hope that maybe in 2022 we might be able to realize that making the life easier for most users should be a priority.

@flying-sheep
Copy link

Wait, so there no way to enable color at all?

@uranusjr
Copy link
Member

I thought FORCE_COLOR would do that (and if nothing is specified you get colouring in a TTY by default). Can someone post a final summary of that conclusion was reached (and what pip needs to do to accomodate it)?

@ssbarnea
Copy link
Contributor

I could do some tests on both local terminals and also on github actions to test if everything is working as expected and if we can close this bug. Still, i kinda find hard to convince pip to produce some colored output when there is nothing to download and I do not want to reset my cache. Is there a specific command I can use to produce something colored?

@willmcgugan
Copy link

If FORCE_COLOR (any value -> on) has emerged as a standard. I could implement that in Rich.

@ssbarnea
Copy link
Contributor

ssbarnea commented May 12, 2022

Hi @willmcgugan! I suppose that FORCE_COLOR/NO_COLOR are now standard enough and at least more-wide spread adopted than hyperlinks support in terminals, which is already supported by rich. If you could do that in rich it would be awesome, especially as most of the other command line tools we use already support it.

@hugovk
Copy link
Contributor Author

hugovk commented Jul 27, 2022

If FORCE_COLOR (any value -> on) has emerged as a standard. I could implement that in Rich.

@willmcgugan This would be great! What's the next step? Would you like an issue created at https://github.com/Textualize/rich?

@willmcgugan
Copy link

Sure.

@ssbarnea
Copy link
Contributor

@hugovk @willmcgugan I am happy to remove that feature from enrich once rich adopts it. With bit of luck, I might even see the day the entire wrapper (enrich) becomes obsolete.

@hugovk
Copy link
Contributor Author

hugovk commented Jul 27, 2022

Posted at Textualize/rich#2425! :)

@webknjaz
Copy link
Member

webknjaz commented Oct 5, 2022

@pradyunsg how about refreshing the vendored copy of rich? Textualize/rich#2449

@pfmoore
Copy link
Member

pfmoore commented Oct 5, 2022

That should happen automatically when we re-vendor prior to the 22.3 release.

@pradyunsg
Copy link
Member

We haven't updated the vendored rich as part of 22.3; since it's presented complexities that we didn't have time for -- see #11063 and #11502 for the discussion around those things.

@pradyunsg
Copy link
Member

See also #11515

ilevkivskyi pushed a commit to python/mypy that referenced this issue Dec 7, 2022
Last time i checked it wasn't the 1960's. So I think the CI could be
colorized.

Configured pytest, tox, mypy(#7771) and pip¹ (I already already
colorized black and isort when I initially added them)


1: Pip doesn't work yet pypa/pip#10909, so
this is just a placedholder for when it (hopefully) soon will.

Co-authored-by: KotlinIsland <kotlinisland@users.noreply.github.com>
@hugovk
Copy link
Contributor Author

hugovk commented Aug 16, 2023

I think we're all set now, thanks all for working on this!

@hugovk hugovk closed this as completed Aug 16, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
project: vendored dependency Related to a vendored dependency type: feature request Request for a new feature
Projects
None yet
Development

No branches or pull requests

10 participants