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

Always pass FORCE_COLOR & NO_COLOR to the environment #3172

Merged
merged 2 commits into from
Jan 12, 2024

Conversation

hashar
Copy link
Contributor

@hashar hashar commented Dec 14, 2023

The environment variables FORCE_COLOR and NO_COLORare a popular way to force or disable color output. An example usage is pytest being run under a CI system.

  • ran the linter to address style issues (tox -e fix)
  • wrote descriptive pull request text
  • ensured there are test(s) validating the fix
  • added news fragment in docs/changelog folder
  • updated/extended the documentation

Copy link
Member

@gaborbernat gaborbernat left a comment

Choose a reason for hiding this comment

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

https://no-color.org/ and FORCE_COLOR are the standard env vars. These app-specific ones don't get to be in the core.

@hashar
Copy link
Contributor Author

hashar commented Dec 14, 2023

I went with PY_COLORS a while ago which was even used by tox at some point ( #163 ). The other use case I had was for pytest but it has learned about NO_COLOR FORCE_COLOR since version 6.0.0 ( pytest-dev/pytest#7466 ).

def should_do_markup(file: TextIO) -> bool:
    if os.environ.get("PY_COLORS") == "1":
        return True
    if os.environ.get("PY_COLORS") == "0":
        return False
    if "NO_COLOR" in os.environ:
        return False
    if "FORCE_COLOR" in os.environ:
        return True
    return (
        hasattr(file, "isatty") and file.isatty() and os.environ.get("TERM") != "dumb"
    )

May I repurpose this pull request to add NO_COLOR and FORCE_COLOR to the environment variables passed by default? I find them handy.

@gaborbernat
Copy link
Member

I don't think you need to add support for them. They should work today. Did you find the opposite?

@hashar
Copy link
Contributor Author

hashar commented Dec 15, 2023

NO_COLOR and FORCE_COLOR are only handled by the config cli parser to set the color flag. For the environments, they are not passed:

[testenv]
commands = python3 -c 'import os; print(os.environ.get("FORCE_COLOR"))'
$ FORCE_COLOR=1 tox -q
None

(same for NO_COLOR)

@gaborbernat
Copy link
Member

If that doesn't work, a pull request is welcome. Just make sure to add the test and the changelog.

@hashar
Copy link
Contributor Author

hashar commented Dec 15, 2023

May you please reopen the pull request or should I send a new one? :)

@gaborbernat gaborbernat reopened this Dec 15, 2023
Copy link
Member

@gaborbernat gaborbernat left a comment

Choose a reason for hiding this comment

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

Don't forget to add test and changelog.

src/tox/tox_env/api.py Outdated Show resolved Hide resolved
@gaborbernat gaborbernat marked this pull request as draft December 15, 2023 20:05
@hashar hashar force-pushed the pass_env=PY_COLORS branch 2 times, most recently from d216ab4 to c4e868b Compare December 17, 2023 20:25
@hashar hashar changed the title Always pass PY_COLORS to the environment Always pass FORCE_COLOR & NO_COLOR to the environment Dec 17, 2023
@hashar
Copy link
Contributor Author

hashar commented Jan 2, 2024

@gaborbernat I have repurposed the pull request to pass pass FORCE_COLOR and NO_COLOR instead of PY_COLORS.

@gaborbernat
Copy link
Member

Yeah, but the CI is failing now that's blocking progress.

@hashar hashar marked this pull request as ready for review January 2, 2024 16:49
@hashar hashar force-pushed the pass_env=PY_COLORS branch 2 times, most recently from c52b728 to 2de4f00 Compare January 3, 2024 07:16
The environment variables `FORCE_COLOR` and `NO_COLOR`are a popular way
to force or disable color output. An example usage is pytest being run
under a CI system.
@hashar
Copy link
Contributor Author

hashar commented Jan 3, 2024

A test was failing under Windows OS.

I also fixed docs/changelog/3171.feature.rst which I forgot to update.

Copy link
Member

@gaborbernat gaborbernat left a comment

Choose a reason for hiding this comment

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

@gaborbernat gaborbernat enabled auto-merge (squash) January 11, 2024 23:52
@gaborbernat gaborbernat merged commit fc80080 into tox-dev:main Jan 12, 2024
25 checks passed
zerario added a commit to zerario/Python-PiShock that referenced this pull request Jan 12, 2024
naa0yama referenced this pull request in naa0yama/PythonBoilerplate Jan 24, 2024
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [tox](https://togithub.com/tox-dev/tox)
([changelog](https://tox.wiki/en/latest/changelog.html)) | `~4.11.0` ->
`~4.12.0` |
[![age](https://developer.mend.io/api/mc/badges/age/pypi/tox/4.12.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/pypi/tox/4.12.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/pypi/tox/4.11.4/4.12.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/pypi/tox/4.11.4/4.12.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>tox-dev/tox (tox)</summary>

### [`v4.12.0`](https://togithub.com/tox-dev/tox/releases/tag/4.12.0)

[Compare
Source](https://togithub.com/tox-dev/tox/compare/4.11.4...4.12.0)

<!-- Release notes generated using configuration in .github/release.yml
at 4.12.0 -->

#### What's Changed

- Exclude bots from generated release notes by
[@&#8203;hugovk](https://togithub.com/hugovk) in
[https://github.com/tox-dev/tox/pull/3163](https://togithub.com/tox-dev/tox/pull/3163)
- Imply `--parallel` when `--parallel-no-spinner` is passed by
[@&#8203;tusharsadhwani](https://togithub.com/tusharsadhwani) in
[https://github.com/tox-dev/tox/pull/3159](https://togithub.com/tox-dev/tox/pull/3159)
- Fix 'open an issue' link in development.rst by
[@&#8203;TheRealFalcon](https://togithub.com/TheRealFalcon) in
[https://github.com/tox-dev/tox/pull/3179](https://togithub.com/tox-dev/tox/pull/3179)
- Fix the CI by [@&#8203;gaborbernat](https://togithub.com/gaborbernat)
in
[https://github.com/tox-dev/tox/pull/3183](https://togithub.com/tox-dev/tox/pull/3183)
- Always pass FORCE_COLOR & NO_COLOR to the environment by
[@&#8203;hashar](https://togithub.com/hashar) in
[https://github.com/tox-dev/tox/pull/3172](https://togithub.com/tox-dev/tox/pull/3172)

#### New Contributors

- [@&#8203;tusharsadhwani](https://togithub.com/tusharsadhwani) made
their first contribution in
[https://github.com/tox-dev/tox/pull/3159](https://togithub.com/tox-dev/tox/pull/3159)
- [@&#8203;TheRealFalcon](https://togithub.com/TheRealFalcon) made their
first contribution in
[https://github.com/tox-dev/tox/pull/3179](https://togithub.com/tox-dev/tox/pull/3179)

**Full Changelog**:
tox-dev/tox@4.11.4...4.12.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/naa0yama/pythonboilerplate).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4xMjcuMCIsInVwZGF0ZWRJblZlciI6IjM3LjEyNy4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants