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

Take the FORCE_COLOR and NO_COLOR environment variables into account #9128

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

stdedos
Copy link
Contributor

@stdedos stdedos commented Oct 7, 2023

Support for NO_COLOR and FORCE_COLOR (and PY_COLORS, as an alias to FORCE_COLOR)
environment variables has been added. When running pylint, the reporter that reports
to stdout will be modified according to the requested mode.

The order is:

  1. NO_COLOR
  2. FORCE_COLOR, PY_COLORS
  3. --output=....

Signed-off-by: Stavros Ntentos [email protected]


  • Document your change, if it is a non-trivial one.
    • A maintainer might label the issue skip-news if the change does not need to be in the changelog.
    • Otherwise, create a news fragment with towncrier create <IssueNumber>.<type> which will be
      included in the changelog. <type> can be one of the types defined in ./towncrier.toml.
      If necessary you can write details or offer examples on how the new change is supposed to work.
    • Generating the doc is done with tox -e docs
  • Relate your change to an issue in the tracker if such an issue exists (Refs incorrect analysing of etree #1234, Closes incorrect analysing of etree #1234)
  • Write comprehensive commit messages and/or a good description of what the PR does.
  • Keep the change small, separate the consensual changes from the opinionated one.
    Don't hesitate to open multiple PRs if the change requires it. If your review is so
    big it requires to actually plan and allocate time to review, it's more likely
    that it's going to go stale.
  • If you used multiple emails or multiple names when contributing, add your mails
    and preferred name in script/.contributors_aliases.json

Type of Changes

Type
✨ New feature

Description

Fixes #3995

Signed-off-by: Stavros Ntentos <[email protected]>
Use the `_is_env_set_and_non_empty`, and the logic decided in the issue,
to modify the reporter(s) list accordingly.

At most, one reporter should be modified - the one going to `stdout`.

Hooking before the `self.set_reporter` was deemed as the smallest incision.

For no particular reason, it was decided that
`_handle_force_color_no_color` would modify its input;
so there would be no need for re-assigning the input we want to modify anyway.

Signed-off-by: Stavros Ntentos <[email protected]>
Signed-off-by: Stavros Ntentos <[email protected]>
Signed-off-by: Stavros Ntentos <[email protected]>
@Pierre-Sassoulas Pierre-Sassoulas added the Enhancement ✨ Improvement to a component label Oct 7, 2023
@github-actions

This comment has been minimized.

@stdedos
Copy link
Contributor Author

stdedos commented Oct 9, 2023

Any ideas of how to detect that a reporter is attached to stdout that works on Windows too? 😓

@jacobtylerwalls jacobtylerwalls changed the title Take the FORCE_COLOR and NO_COLOR environnement variables into account Take the FORCE_COLOR and NO_COLOR environement variables into account Oct 28, 2023
@jacobtylerwalls jacobtylerwalls changed the title Take the FORCE_COLOR and NO_COLOR environement variables into account Take the FORCE_COLOR and NO_COLOR environment variables into account Oct 28, 2023

This comment has been minimized.

@hugovk
Copy link
Contributor

hugovk commented Jun 7, 2024

÷1 to FORCE_COLOR and NO_COLOR.

I suggest to leave PY_COLORS out because it's pytest specific. (py is there internal helper library and iirc it originated from there.)

https://docs.pytest.org/en/stable/reference/reference.html#envvar-PY_COLORS

Copy link
Contributor

🤖 According to the primer, this change has no effect on the checked open source code. 🤖🎉

This comment was generated for commit 51950fe

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement ✨ Improvement to a component
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Take the FORCE_COLOR and NO_COLOR environnement variables into account incorrect analysing of etree
3 participants