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 the NO_COLOR environment variable #356

Merged
merged 14 commits into from Apr 20, 2022
Merged

Support the NO_COLOR environment variable #356

merged 14 commits into from Apr 20, 2022

Conversation

akaihola
Copy link
Owner

@akaihola akaihola commented Apr 10, 2022

Setting NO_COLOR to any non-empty value will disable syntax highlighting even on terminal output. This overrides the color = true setting in pyproject.toml, but will be overridden by PY_COLORS=1 or the --color command line option, if those are present.

Fixes #355.

Previously, #353 added a command line options, a pyproject.toml option and the PY_COLORS environment variable to for enabling/disabling syntax highlighting.

See earlier discussion in #353, and pypa/pip#10909, for the reasoning behind supporting both NO_COLOR and PY_COLORS.

  • decide whether it would make sense to support FORCE_COLOR as well (see this comment in pypa/pip)
  • add suppert for FORCE_COLOR
  • empty value of NO_COLOR and FORCE_COLOR should be considered as activating that option, similar to how Pytest works
  • tests
  • ensure fast performing tests
  • change log
  • code review

@akaihola
Copy link
Owner Author

akaihola commented Apr 10, 2022

@roniemartinez & @MatthijsBurgh, in addition to adding support for the NO_COLOR environment variable, I've now converted the tests for syntax highlighting configuration into parametrized matrix tests as much as possible.

There are now four test functions:

To reduce duplication and boilerplate, I've created some parametrized fixtures instead of straight using @pytest.mark.parametrize. This way the patching of environment variables, creating the pyproject.toml file and patching the return value of sys.stdout.isatty are done inside the fixtures instead of separately in each test function.

I still suspect that for someone fresh looking at the tests, the resulting 256 lines of Python code are more difficult to understand and maintain than the 254 lines (here still just 146 lines since NO_COLOR wasn't yet implemented) it would take by enumerating each parameter combination separately.

@akaihola akaihola marked this pull request as ready for review April 17, 2022 17:42
Copy link
Collaborator

@roniemartinez roniemartinez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Akaihola's Open source work automation moved this from In progress to Reviewer approved Apr 19, 2022
@akaihola akaihola merged commit 9033d01 into master Apr 20, 2022
Akaihola's Open source work automation moved this from Reviewer approved to Done Apr 20, 2022
@akaihola akaihola deleted the no-color-env-var branch April 20, 2022 19:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Development

Successfully merging this pull request may close these issues.

Add support for the NO_COLOR environment variable
2 participants