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

Add support for NO_COLOR and FORCE_COLOR #7466

Merged
merged 7 commits into from Jul 10, 2020
Merged

Add support for NO_COLOR and FORCE_COLOR #7466

merged 7 commits into from Jul 10, 2020

Conversation

hugovk
Copy link
Member

@hugovk hugovk commented Jul 8, 2020

Fixes #7464.

PY_COLORS is pytest-specific and comes first (#7464 (comment)).

Then NO_COLOR takes precendece over FORCE_COLOR (like tartley/colorama#230).

Then check the other rules.

Also includes the Jython removal from #7462.

@hugovk hugovk changed the title Add no color and force color Add support for NO_COLOR and FORCE_COLOR Jul 8, 2020
Copy link
Member

@nicoddemus nicoddemus left a comment

Choose a reason for hiding this comment

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

Looks good!

Would you mind add a small explanation to those variables and PY_COLORS to reference.rst under Environment Variables? Thanks!

@hugovk
Copy link
Member Author

hugovk commented Jul 9, 2020

Would you mind add a small explanation to those variables and PY_COLORS to reference.rst under Environment Variables? Thanks!

Added!

Should they be in alphabetical order? For example, before:

  • PYTEST_ADDOPTS
  • PYTEST_DEBUG
  • PYTEST_PLUGINS
  • PYTEST_DISABLE_PLUGIN_AUTOLOAD
  • PYTEST_CURRENT_TEST
  • PY_COLORS
  • NO_COLOR
  • FORCE_COLOR

After:

  • FORCE_COLOR
  • NO_COLOR
  • PYTEST_ADDOPTS
  • PYTEST_CURRENT_TEST
  • PYTEST_DEBUG
  • PYTEST_DISABLE_PLUGIN_AUTOLOAD
  • PYTEST_PLUGINS
  • PY_COLORS

doc/en/reference.rst Outdated Show resolved Hide resolved
doc/en/reference.rst Outdated Show resolved Hide resolved
@nicoddemus
Copy link
Member

Should they be in alphabetical order?

Good catch, I think so, perhaps except keeping the pytest-specific ones separated from the others:

  • PYTEST_ADDOPTS
  • PYTEST_CURRENT_TEST
  • PYTEST_DEBUG
  • PYTEST_DISABLE_PLUGIN_AUTOLOAD
  • PYTEST_PLUGINS

Also the non-pytest specific could be order by precedence perhaps?

  • PY_COLORS
  • NO_COLOR
  • FORCE_COLOR

What do you think?

@hugovk
Copy link
Member Author

hugovk commented Jul 9, 2020

Done!

Copy link
Member

@nicoddemus nicoddemus left a comment

Choose a reason for hiding this comment

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

Awesome, thanks @hugovk!

@nicoddemus nicoddemus merged commit c1c5a2b into pytest-dev:master Jul 10, 2020
@nicoddemus
Copy link
Member

Thanks @hugovk!

@hugovk hugovk deleted the add-NO_COLOR-and-FORCE_COLOR branch July 10, 2020 11:53
@@ -0,0 +1,3 @@
Added support for ``NO_COLOR`` and ``FORCE_COLOR`` environment variables to control colored output.

For more information, see `the docs <https://docs.pytest.org/en/stable/reference.html#environment-variables>`__.
Copy link
Contributor

Choose a reason for hiding this comment

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

This could/should use an internal reference (Python uses using-on-envvars).
In general (internal) documentation links should not go through external resources/protocols.

Copy link
Member

Choose a reason for hiding this comment

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

True, that escaped my mind, thanks. 👍

Copy link
Member

Choose a reason for hiding this comment

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

Done: #7474

Thanks again

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 this pull request may close these issues.

Support NO_COLOR to disable colored output
3 participants