tty: treat empty `NO_COLOR` same as absent `NO_COLOR`
As specified in https://no-color.org/, setting the env variable to an empty string should result in that env variable to be ignored:
Command-line software which adds ANSI color to its output by default should check for a
NO_COLORenvironment variable that, when present and not an empty string (regardless of its value), prevents the addition of ANSI color.
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 90.07%. Comparing base (
eaebfab) to head (7c978b8). Report is 1 commits behind head on main.
Additional details and impacted files
@@ Coverage Diff @@
## main #58074 +/- ##
==========================================
- Coverage 90.09% 90.07% -0.02%
==========================================
Files 640 640
Lines 188450 188450
Branches 36966 36962 -4
==========================================
- Hits 169789 169752 -37
- Misses 11364 11396 +32
- Partials 7297 7302 +5
| Files with missing lines | Coverage Δ | |
|---|---|---|
| lib/internal/tty.js | 99.23% <100.00%> (ø) |
:rocket: New features to boost your workflow:
- :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
- :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
@BridgeAR is it semver-major or just a fix? Wdyt about not landing it on LTS release lines to minimize the risk of ecosystem breakage, but still land it on Current release line?
@aduh95 I think it's fine to land it on 24 or do you mean any release before?
Commit Queue failed
- Loading data for nodejs/node/pull/58074 ✔ Done loading data for nodejs/node/pull/58074 ----------------------------------- PR info ------------------------------------ Title tty: treat empty `NO_COLOR` same as absent `NO_COLOR` (#58074) ⚠ Could not retrieve the email or name of the PR author's from user's GitHub profile! Branch aduh95:fix-no-color -> nodejs:main Labels tty, author ready, needs-ci, dont-land-on-v20.x, dont-land-on-v22.x, dont-land-on-v23.x Commits 4 - tty: treat empty `NO_COLOR` same as absent `NO_COLOR` - fixup! tty: treat empty `NO_COLOR` same as absent `NO_COLOR` - Revert "fixup! tty: treat empty `NO_COLOR` same as absent `NO_COLOR`" - fixup! tty: treat empty `NO_COLOR` same as absent `NO_COLOR` Committers 1 - Antoine du Hamel <[email protected]> PR-URL: https://github.com/nodejs/node/pull/58074 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> ------------------------------ Generated metadata ------------------------------ PR-URL: https://github.com/nodejs/node/pull/58074 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> -------------------------------------------------------------------------------- ℹ This PR was created on Tue, 29 Apr 2025 10:23:00 GMT ✔ Approvals: 3 ✔ - Luigi Pinca (@lpinca): https://github.com/nodejs/node/pull/58074#pullrequestreview-2803306357 ✔ - Michaël Zasso (@targos) (TSC): https://github.com/nodejs/node/pull/58074#pullrequestreview-2803318306 ✔ - Ruben Bridgewater (@BridgeAR) (TSC): https://github.com/nodejs/node/pull/58074#pullrequestreview-2813243352 ✘ Last GitHub CI failed ✘ No Jenkins CI runs detected -------------------------------------------------------------------------------- ✔ Aborted `git node land` session in /home/runner/work/node/node/.ncuhttps://github.com/nodejs/node/actions/runs/14811753178
CI: https://ci.nodejs.org/job/node-test-pull-request/66566/
CI: https://ci.nodejs.org/job/node-test-pull-request/66660/
@aduh95 this has some conflicts. It could otherwise likely land.
CI: https://ci.nodejs.org/job/node-test-pull-request/67698/
CI: https://ci.nodejs.org/job/node-test-pull-request/67700/
Landed in afbf2f385a0d8f9202ed039de5dd8ba204a47ea5