util: downgrade hex colors to terminal color depth - #64955
Conversation
nodejs-github-bot
added
needs-ci
styleText() always emitted 24-bit TrueColor sequences for hex colors, even when the terminal only supports 16 or 256 colors. Terminals that do not understand the sequence render it as literal text or drop the color. Emit the sequence matching the color depth reported by the stream, or by FORCE_COLOR when it is set: TrueColor for level 3, the closest entry of the 256-color palette for level 2, and the closest of the 16 basic colors for level 1. Named formats are unaffected, as they already are basic color codes. When validateStream is false there is no stream to inspect, so the color is only downgraded if FORCE_COLOR is set. The conversions mirror the ones used by ansis and ansi-styles, so a hex color downgrades to the same color the rest of the ecosystem picks. Refs: https://nodejs.org/api/cli.html#force_color1-2-3 Signed-off-by: Tobbe Lundberg <tobbe@tlundberg.com>
daf0deb to
931d263
Compare
avivkeller
added
the
semver-major
avivkeller
removed
the
semver-major
|
|
||
| // Hex colors are downgraded to the color depth reported by `FORCE_COLOR`, so | ||
| // make sure the environment running the test does not set it. | ||
| delete process.env.FORCE_COLOR; |
There was a problem hiding this comment.
happy to be wrong on this but in my own practice I generally avoid delete as the education I've received has been that it's bad to use and that's been reinforced by code style tooling I've always used.
It doesn't seem like we use it anywhere else in the codebase in this way. I don't have a direct recommendation on what to replace it with, but I'm cautious to see us introduce it as a precedent with this PR.
There was a problem hiding this comment.
I worked around it with a spread assignment. The if can be dropped if you like, but the linter complains about an unused variable if I do, so I'd have to add a comment to disable the linter in that case. I'm happy with either solution.
There was a problem hiding this comment.
But I also did find a few delete process... in the code base already, like here:
So it might be okay to just go with the simpler delete usage.
| function styled(format, forceColor) { | ||
| process.env = { ...originalEnv }; | ||
| if (forceColor === undefined) { | ||
| delete process.env.FORCE_COLOR; |
There was a problem hiding this comment.
as as my previous comment about delete usage.
There was a problem hiding this comment.
Worked around this delete as well with a spread instead
4 participants
styleText()emits 24-bit TrueColor sequences for hex colors regardless of what the terminal supports, soFORCE_COLOR=1andFORCE_COLOR=2, which document 16-color and 256-color support, get a sequence their terminal may render as literal text or ignore.This makes the hex path honor the color depth that is already computed for the stream:
#ffcc00FORCE_COLOR=3, or a 16m-color terminal\e[38;2;255;204;0mFORCE_COLOR=2, or a 256-color terminal\e[38;5;220mFORCE_COLOR=1, or a 16-color terminal\e[93mFORCE_COLOR=0,NO_COLOR, non-TTYNamed formats are unaffected: they already are basic color codes. When
validateStreamisfalsethere is no stream to inspect, so the color is only downgraded ifFORCE_COLORis set, and stays 24-bit otherwise.Implementation notes:
internal/util/colorsgainsgetColorDepth(stream)andgetForcedColorDepth().shouldColorize()becomes a> 2check on the former, keeping its previous behavior.ansisandansi-styles, so a hex color downgrades to the same color the rest of the ecosystem picks for it. Verified by comparing againstansis/src/color-math.jsover all 16777216 colors: zero differences at both depths.I don't know enough about color math to really understand the conversions used here. I pretty much copied the implementation ansis uses.
EDIT: I took a second look at the Node docs, and it's pretty clear that
FORCE_COLOR=0should disable colors:So I updated the implementation. But do note two things:
styleText('red', ...)). So those are not disabled by settingFORCE_COLOR=0Assisted-by: claude-opus-5, deep-seek-v4-flash-0731, kimi-k3