node icon indicating copy to clipboard operation
node copied to clipboard

tty: treat empty `NO_COLOR` same as absent `NO_COLOR`

Open aduh95 opened this issue 8 months ago • 5 comments

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_COLOR environment variable that, when present and not an empty string (regardless of its value), prevents the addition of ANSI color.

aduh95 avatar Apr 29 '25 10:04 aduh95

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%> (ø)

... and 33 files with indirect coverage changes

: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.

codecov[bot] avatar Apr 29 '25 12:04 codecov[bot]

@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 avatar Apr 29 '25 13:04 aduh95

@aduh95 I think it's fine to land it on 24 or do you mean any release before?

BridgeAR avatar Apr 30 '25 01:04 BridgeAR

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/.ncu
https://github.com/nodejs/node/actions/runs/14811753178

nodejs-github-bot avatar May 03 '25 14:05 nodejs-github-bot

CI: https://ci.nodejs.org/job/node-test-pull-request/66566/

nodejs-github-bot avatar May 03 '25 16:05 nodejs-github-bot

CI: https://ci.nodejs.org/job/node-test-pull-request/66660/

nodejs-github-bot avatar May 06 '25 22:05 nodejs-github-bot

@aduh95 this has some conflicts. It could otherwise likely land.

BridgeAR avatar Jun 02 '25 00:06 BridgeAR

CI: https://ci.nodejs.org/job/node-test-pull-request/67698/

nodejs-github-bot avatar Jun 28 '25 08:06 nodejs-github-bot

CI: https://ci.nodejs.org/job/node-test-pull-request/67700/

nodejs-github-bot avatar Jun 28 '25 11:06 nodejs-github-bot

Landed in afbf2f385a0d8f9202ed039de5dd8ba204a47ea5

aduh95 avatar Jun 28 '25 13:06 aduh95