util: add %c to ANSI transform for console.log()

[jcbhmr]

this is something that I saw already had a closed issue #29605 and I figured I should put my code where my mouth is and open a PR instead of waiting for a long-dead issue response. idk if i should open a new issue instead of a pr first?

I'm also not sure of the policy of nodejs around copying code from other places like from deno. deno already has this %c => ansi thing "solved" so i copied some code from there. idk if this is allowed or what. answered in comment

relevant deno bits:

• small basic CSS prop splitter https://github.com/denoland/deno/blob/ece2a3de5b19588160634452638aa656218853c5/ext/console/01_console.js#L2821
• CSS to ansi https://github.com/denoland/deno/blob/ece2a3de5b19588160634452638aa656218853c5/ext/console/01_console.js#L2933
• the actual %c substitution https://github.com/denoland/deno/blob/ece2a3de5b19588160634452638aa656218853c5/ext/console/01_console.js#L3115

i would think this is a good improvement idea but idk it was shot down in #29605 due to complexity concerns.

---

fix #52350

[bnoordhuis]

deno already has this %c => ansi thing "solved" so i copied some code from there. idk if this is allowed or what

It is, provided it's compatible with node's MIT license (which deno is also licensed under so that's okay.)

also is this %c support something that nodejs actually wants?

Good question. I personally don't see the value and very few people ever feature-requested it.

[jcbhmr]

i think it works!

[jcbhmr]

📫 pinging previous participants from #29605 @humphd @jasnell @devsnek

[humphd]

@jcbhmr very cool, thanks for pushing this forward!

[jcbhmr]

its been a month lol 😅 what would be the best way to go about getting feedback/progress on this PR? do i need to @ someone? what should i do? anything? just wait? 🤷‍♀️ idk

[Uzlopak]

@nodejs/util

[ljharb]

(went ahead and filed denoland/deno#20856 to upstream some of these suggestions)

[jasnell]

I'm +1 on this moving forward but this needs docs.

[avivkeller]

If implemented, this (IMO) would be a semver-minor change, as it adds a new control code to the console (%c).

I curious for a review from the team, as it's been several months since this was first made, and it's possible there are changes that need to be made

Tip

I'm a member of the triage team, and not a core collaborator.

[avivkeller]

Tip

I'm a member of the triage team, and not a core-collaborator. While my approval shows my support, it holds no power.

[aduh95]

Can you please rebase and fix the linter errors please?

lib/internal/util/inspect_colors.js
   5:3  error  Out of ASCIIbetical order - SafeMap >= RegExpPrototypeExec                node-core/alphabetize-primordials
   6:3  error  Out of ASCIIbetical order - RegExpPrototypeExec >= NumberParseInt         node-core/alphabetize-primordials
   7:3  error  Out of ASCIIbetical order - NumberParseInt >= MathMax                     node-core/alphabetize-primordials
   9:3  error  Out of ASCIIbetical order - MathMin >= MathAbs                            node-core/alphabetize-primordials
  12:3  error  Out of ASCIIbetical order - StringPrototypeTrim >= ArrayPrototypePush     node-core/alphabetize-primordials
  13:3  error  Out of ASCIIbetical order - ArrayPrototypePush >= ArrayPrototypeIncludes  node-core/alphabetize-primordials
  15:3  error  Out of ASCIIbetical order - StringPrototypeSplit >= ArrayIsArray          node-core/alphabetize-primordials

[jcbhmr]

@aduh95 i have rebased against upstream/main and fixed the lint-js errors (I did it manually; is there an automated way like lint --fix or something?) and squashed all commits into one

[avivkeller]

@aduh95 i have rebased against upstream/main and fixed the lint-js errors (I did it manually; is there an automated way like lint --fix or something?) and squashed all commits into one

AFAIK make lint-js-fix

[nodejs-github-bot]

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

[aduh95]

I think this should be using utile.styleText instead of reimplementing it. Unless there's a good reason not to use it? If so we should add a comment.

[jcbhmr]

util.styleText() uses util.inspect.colors.

#49205 (comment)

after I tried digging into it more I don't think that using util.inspect.colors is a good idea for console.log() %c.

1. util.inspect.colors is mutable. this can be good and bad. on one hand it lets users customize what "red" means. on the other had, it lets users change what "red" means but NOT what #ff0000 means. changing what "red" means is something not even browsers let you do (to my knowledge)!
2. I don't think it would be a good idea to pull out just the magic ~15 or so color names and have those reference some object defined in a different file which every other color (like "salmon") uses color->hex translation. it creates two files to dig through to follow instead of sticking everything in one spot.

ex for number 1:

util.inspect.colors.red = util.inspect.colors.green
console.log("%cI am red!", "color:red") // what color am I?
console.log("%cI am red!", "color:#ff0000") // what color am I?

should these print the same color? or different? what about salmon:

// should attaching a new property to util.inspect.colors
// add a new CSS->ANSI color translation?
util.inspect.colors.salmon = util.inspect.colors.green
console.log("%cI am red!", "color:salmon") // what color am I?
console.log("%cI am red!", "color:#fa8072") // what color am I?

util.inspect.colors is mutable by the user which is a whole can of worms. do you make a copy of it? if so, why not just define the literals in the file; why reference util.inspect.colors at all for ~15 color codes -- its almost easier to just define the object in the same file lol.

for number 2 to elaborate a bit... i also don't think its a good idea to deviate too far from deno's original code so that its easier to copy-paste anyu changes in the intermediate future and say "thanks deno devs!" instead of grafting any updates onto custom node.js util.inspect.colors in-another-file code.

relevant code bits

jcbhmr/node@c130398/lib/internal/util/inspect_colors.js#L20

jcbhmr/node@c130398/lib/internal/util/inspect_colors.js#L373

so tldr I don't think deviating from deno's code (which is actually totally unchanged in the color section that we are discussing; they only modified the other formatting-related code, not the CSS color %c magic) is a good idea.

tldr of the quote:

1. util.styleText uses util.inspect.colors. util.inspect.colors is mutable. this is bad because some colors (red, green, blue) are user mutable while others (salmon, deepskyblue, #ff0000) are not mutable.
2. I don't think its a good idea to split color CSS->ANSI processing over two different modules/files for maintenance and readability and copy-paste-from-deno-ability to save only ~20 lines of if statements

[aduh95]

FWIW I don't find the arguments for not using util.inspect.colors very compelling: I don't think we should be worry about user mutability (on the contrary, it can be a plus from an accessibility perspective), and from a maintenance perspective having one list of colors is better than two.

[jcbhmr]

@aduh95
cc @redyetidev

it can be a plus from an accessibility perspective

I've seen this mentioned before about how "mutable colors are good" and I want to refute this.

1. It's unreasonable to expect any end-user who is colorblind or has a vision impairment to use node --import 'data:text/javascript,import { inspect } from "node:util"; inspect.colors.red = inspect.colors.gray' node_modules/myapp/bin/cli.js. Or even NODE_OPTIONS='--import "data:text/javascript,..."'. That's just plain not portable to the numerous other CLI applications like make, ripgrep, cargo, etc.; it only works on Node.js apps that are using the builtin inspect.colors mapping and don't pull in https://www.npmjs.com/package/ansi-colors or similar npm packages which define their own copies. Editing the "which ANSI code is for red?" information as an end user is completely unfeasible and so high effort/reward to be overlooked in favor of broader terminal settings or system visual settings.

2. Not all apps are Node.js apps where this would apply. ls, clang, cargo, etc. are all non-Node.js. Instead, a user will probably use a terminal emulator which custom-interprets ANSI color codes and remaps them at the terminal level. The number one reason you would want to do this is color blindness. Apps often have ways to force no-color text per-app either with flags or NO_COLOR or TERM=dumb or mycmd | cat or similar. Another more global way is disabling ANSI color codes in your terminal emulator or forcing a minimum contrast like iTerm does https://iterm2.com/documentation-preferences-profiles-colors.html#:~:text=faint%22%20text%20is.-,Minimum%20Contrast,-Sometimes%20text%20in which when maxed effectively disables ANSI color codes. In Windows Terminal you use https://learn.microsoft.com/en-us/windows/terminal/customize-settings/color-schemes#creating-your-own-color-scheme to set a custom alias for the common (non hex code) colors to suit your needs. Ubuntu terminal can redefine ANSI colors https://itsfoss.com/change-terminal-color-ubuntu/ as well. VS Code also offers some settings for its integrated terminal https://code.visualstudio.com/docs/terminal/appearance#_terminal-colors

3. Most apps want to be sure that they are using that color. From a developer perspective having something that's mutable depending on whether you do const red = inspect.colors.red[0] or const getRed = () => inspect.colors.red[0] is a bit baffling. You generally don't want to encourage mutating such a critical core color map. Especially since it will become out of sync with other npm packages which maintain their own ANSI map like https://www.npmjs.com/package/ansi and now you have two different definitions for the color green. Unlikely (I hope) but not improbable. It's unexpected to be able to mutate a color map. Rust, Go, C++, Java, etc. don't do mutable color maps. It violates the Principle of least astonishment; you'd expect color:red to always print \e[0;31m, right?

4. In the Real World™ inspect.colors mutation doesn't seem to be used for accessibility. Or much at all for that matter. From a GitHub search https://github.com/search?q=%2Finspect%5C.colors%5B%5C%5B%5C.%5D%5Cw%2B%5C%5D%3F%5Cs*%3D%2F&type=code for /inspect\.colors[\[\.]\w+\]?\s*=/ there are only eight results, only one of which actually overwrites a color to 0, 0, 0 for a test. Sourcegraph search for the same https://sourcegraph.com/search?q=context:global+/inspect%5C.colors%5B%5C%5B%5C.%5D%5Cw%2B%5C%5D%3F%5Cs*%3D/&patternType=keyword&sm=0 yields 37 results across 6 files. Same thing; defining new colors and DefinitelyTyped test. The preferred way for applications to allow color customization seems to be through environment variables like LS_COLORS & MYCLI_THEME=catppuccin or CLI flags like --theme=light. Or NO_COLOR=1 to disable output https://no-color.org/

5. Node.js recommends using the higher-level inspect.styles to choose custom colors instead of mutating the global inspect.colors map. https://nodejs.org/api/util.html#customizing-utilinspect-colors 😅

6. Browsers are up there with most widely used software ever and do not allow developers to customize what color: red; background-color: blue means. Instead they provide the user with tools to view the page their own way at the viewer level similar to how a terminal emulator provides knobs to customize terminal colors. Like turning off JavaScript https://www.impressivewebs.com/how-to-disable-javascript-in-almost-any-browser/ or disabling CSS stylesheets https://smallbusiness.chron.com/disable-firefox-stylesheets-49055.html or even (albiet with extensions) overlaying custom styles https://darkreader.org/ or my favorite: https://chromewebstore.google.com/detail/colorblind-dalton-for-goo/afcafnelafcgjinkaeohkalmfececool <-- this kind of "apply a filter on top of the application" or "use app settings, don't redefine constants" is a good idiom to follow IMO.

While I agree we are stuck with a mutable util.inspect.colors*, I don't think actively citing "mutating util.inspect.colors is an opportunity for accessibility since it's a centralized map that can be customized" is a good thing to tell developers. There are other better & higher-level dev-friendly ways to improve accessibility without messing with color name/value pairs.

^{*inspect.colors.MYCOLOR=...; inspect.style.number="MYCOLOR" since inspect.style.* uses inspect.color.* properties -- is still needed}

---

and from a maintenance perspective having one list of colors is better than two.

^{I don't think the ANSI core color codes are going to be extended or added to any time soon lol 😉}

The main maintenance I can imagine is copy-pasting code from https://github.com/denoland/deno/blob/main/ext/console/01_console.js#L2449-L2931 which is a pretty easy thing to do right now; it's all the same code (with some primordial tweaks)! If you start changing more stuff, it becomes more of a hassle. 🤷‍♀️

---

If this code:

if (css.backgroundColor == null) {
  ansi += '\x1b[49m';
} else if (css.backgroundColor === 'black') {
  ansi += '\x1b[40m';
} else if (css.backgroundColor === 'red') {
  ansi += '\x1b[41m';
} else if (css.backgroundColor === 'green') {
  ansi += '\x1b[42m';
} else if (css.backgroundColor === 'yellow') {
  ansi += '\x1b[43m';
} else if (css.backgroundColor === 'blue') {
  ansi += '\x1b[44m';
} else if (css.backgroundColor === 'magenta') {
  ansi += '\x1b[45m';
} else if (css.backgroundColor === 'cyan') {
  ansi += '\x1b[46m';
} else if (css.backgroundColor === 'white') {
  ansi += '\x1b[47m';
} else if (ArrayIsArray(css.backgroundColor)) {
  const { 0: r, 1: g, 2: b } = css.backgroundColor;
  ansi += `\x1b[48;2;${r};${g};${b}m`;
} else {
  const parsed = parseCssColor(css.backgroundColor);
  if (parsed !== null) {
    const { 0: r, 1: g, 2: b } = parsed;
    ansi += `\x1b[48;2;${r};${g};${b}m`;
  } else {
    ansi += '\x1b[49m';
  }
}

were changed to this:

if (css.backgroundColor == null) {
  ansi += '\x1b[49m';
} else if (css.backgroundColor === 'black') {
  ansi += `\x1b[${inspect.colors.bgBlack[0]}m`;
} else if (css.backgroundColor === 'red') {
  ansi += `\x1b[${inspect.colors.bgRed[0]}m`;
} else if (css.backgroundColor === 'green') {
  ansi += `\x1b[${inspect.colors.bgGreen[0]}m`;
} else if (css.backgroundColor === 'yellow') {
  ansi += `\x1b[${inspect.colors.bgYellow[0]}m`;
...

or

if (css.backgroundColor == null) {
  ansi += '\x1b[49m';
} else if (ArrayPrototypeIncludes(['black', 'red', 'green', 'yellow', 'blue', 'magenta', 'cyan', 'white'], css.backgroundColor)) {
  const upperBg = stringPrototypeToUpperCase(css.backgroundColor[0]) + StringPrototypeSlice(css.backgroundColor, 1);
  ansi += `\x1b[${inspect.color[`bg${upperBg}`][0]}m`;
} else {
  const parsed = parseCssColor(css.backgroundColor);
  if (parsed !== null) {
    const { 0: r, 1: g, 2: b } = parsed;
    ansi += `\x1b[48;2;${r};${g};${b}m`;
  } else {
    ansi += '\x1b[49m';
  }
}

I don't think that unifies things the way one would hope. There's still the massive Map<string, string> holding all the salmon=>#FF8C69 color names/values; should that be stuffed into inspect.colors too? I don't know. The crux is that the inspect.colors map doesn't include all the colors that CSS colors need, while at the same time also being mutable. Where does the line between internal color map and user-mutable inspect.colors map get drawn?

In the way the code is above ☝ the core ANSI colors (background:red, color:blue) are user-mutable vs but CSS colors (salmon, deepskyblue) or hex colors (#223344) aren't. ...which is weird and doesn't feel right to me. I'd prefer uniformity and a guarantee of "I say color:red, I get red same as if I said color:#ff0000; the terminal user can NO_COLOR=1 or MYAPP_THEME=colorblind2 or edit their terminal settings if they need to." as a developer.

@aduh95 I can rewrite those 20 if statement lines. I just need some feedback as to how to rewrite the code to use inspect.colors:

• Which ANSI code should this code use? inspect.colors.salmon = inspect.colors.red; console.log("%cfoo", "color:salmon")
• Which ANSI code should this code use? inspect.colors.red = inspect.colors.green; console.log("%cfoo, "color:#ff0000")
• Which ANSI code should this code use? Should it throw? delete inspect.colors.red; console.log("%cfoo", "color:red")
• Which ANSI code should this code use? Should it throw? inspect.colors.red = null; console.log("%cfoo", "color:red")
• Which ANSI code should this code use? console.log("%cfoo", "color:bgBlue")
• Which ANSI code should this code use? console.log("%cfoo", "color:hasOwnProperty")
• What's the current behaviour for util.format() when a inspect.colors is an invalid type?
• Should console.log() ever be able to throw an error?
• Which ANSI code should this code use? inspect.colors["#111111"]=inspect.colors.red; console.log("%cfoo", "color:#111111")

[avivkeller]

Wow! That is detailed! IMO your current implementation works, and it can always be changed later.

In the long term, what if (and this is only a theoretical idea), in a seperate PR (after this lands), util.colors was extended to include the colors listed here, and this can pull from both hexes and the util.colors map?

I'm happy to implement that once this lands.

[aduh95]

4. In the Real World™ inspect.colors mutation doesn't seem to be used for accessibility. Or much at all for that matter. From a GitHub search https://github.com/search?q=%2Finspect%5C.colors%5B%5C%5B%5C.%5D%5Cw%2B%5C%5D%3F%5Cs*%3D%2F&type=code for /inspect\.colors[\[\.]\w+\]?\s*=/ there are only eight results

That seems to further confirm that mutability is a non-problem 🤷‍♂️

3. Most apps want to be sure that they are using that color

The user is the one reading the text in the end, so it's good that they can control/tweak it, and how the app author feels about that seems irrelevant to me.

should that be stuffed into inspect.colors too?

Yes, like I said, 1 list is better than 2.

• Should console.log() ever be able to throw an error?

In an ideal world, no (note that it's already not the case, it's possible that calling console.log result in an error when some internals have been mutated, but IMO we should not add more cases where that happens).

• Which ANSI code should this code use? inspect.colors.salmon = inspect.colors.red; console.log("%cfoo", "color:salmon")
• Which ANSI code should this code use? inspect.colors.red = inspect.colors.green; console.log("%cfoo, "color:#ff0000")
• Which ANSI code should this code use? Should it throw? delete inspect.colors.red; console.log("%cfoo", "color:red")
• Which ANSI code should this code use? Should it throw? inspect.colors.red = null; console.log("%cfoo", "color:red")
• Which ANSI code should this code use? inspect.colors["#111111"]=inspect.colors.red; console.log("%cfoo", "color:#111111")

I think it's fine to assume inspect.colors won't be mutated, and so it's fine to have undefined behavior.

• Which ANSI code should this code use? console.log("%cfoo", "color:bgBlue")

Given it's not valid CSS, I think it's fine if it has undefined behavior (e.g. if it behaves similar to background-color:blue).

• Which ANSI code should this code use? console.log("%cfoo", "color:hasOwnProperty")

hasOwnProperty is not a property of inspect.colors.

---

This is just my opinion, feel free to disagree, I'm not a user of that feature anyway, and my comments are non-blocking.

[avivkeller]

While I like the idea of one list rather than two, my only concern is that there is no background vs foreground on the util colors list (bgBlue etc, but that's not the same).

I think that this PR looks okay for now, and once it lands a more long term change to util colors and this might be a valuable thing to discuss.

Then again, I'm a triage member, so I don't really have an opinion on this.

[jcbhmr]

@aduh95 to clarify: my 6-point rant wasn't really about how it being mutable was bad but rather that "mutable colors are good because accessibility" is a bit disingenuous. I was pushing against that tangent 😜

I think it's fine to assume inspect.colors won't be mutated, and so it's fine to have undefined behavior.

Thanks for clarifying; I was confused earlier about encouraging mutating colors for accessibility reasons. 👍

Do you have any ideas for how to merge the inspect.colors and the color keywords map?

While I like the idea of one list rather than two, my only concern is that there is no background vs foreground on the util colors list (bgBlue etc, but that's not the same).

☝ This is a big one. Supported CSS properties are color, background-color,
font-weight, font-style, text-decoration, text-decoration-color, and
text-decoration-line. You almost want to define a different CSS->ANSI Record<string, string> for each one. I don't really know how you want to model all of these constants in a centralized way that makes sense. You could derive or generate inspect.colors at runtime from some other higher-level CSS->ANSI construct. idk 🤷‍♀️

A way to do this might be to define inspect.colors in terms of console.log() %c instead of defining console.log() %c in terms of inspect.colors plus the cssColorKeywords map?

const RESET_FOREGROUND = 39;
const RESET_BACKGROUND = 49;
const RESET_ALL = 0;
const RESET_ITALIC = 23;
inspect.colors = {
  // \x1B[31m\x1B[0m
  red: [+StringPrototypeSlice(format("%c", "color:red"), 2, -5), RESET_FOREGROUND],
  blue: [+StringPrototypeSlice(format("%c", "color:blue"), 2, -5), RESET_FOREGROUND],
  bgRed: [+StringPrototypeSlice(format("%c", "background:red"), 2, -5), RESET_BACKGROUND],
  bgBlue: [+StringPrototypeSlice(format("%c", "background:blue"), 2, -5), RESET_BACKGROUND],
  italic: [+StringPrototypeSlice(format("%c", "font-style:italic"), 2, -5), RESET_ITALIC],
  // could use regex instead of manual slicing idk
  // could also generate the js inspect.colors constants at buildtime as part of the makefile
  // or in a 'go generate'-like way to regenerate them ondemand but still track the generated result in git
  // could also use a `get colors() {}` getter to generate the map once ondemand and then cache it instead of eagerly
  // total tangent: this would be a great usecase for https://bun.sh/blog/bun-macros
}

This would remove the desire to stuff things like salmon and deepskyblue into inspect.colors and centralize all the CSS and ANSI stuff in the inspect_colors.js instead of in inspect.colors constants where its hard to fit everything. Just a thought.

[avivkeller]

Maybe, but that's an issue for a different PR. Let's focus on this one landing first.

IMO once this lands it might be a reason to discuss the possible overhaul of util.inspect.colors, but thats a discussion that isn't my call.

[avivkeller]

👋 Hey, this has been author ready for a little while. WDYT about making a decision whether or not to consider landing it?

[jasnell]

Minor nit... can populating this map be done lazily? Just concerned that this is a fairly large number of strings to create when they are only used in certain situations.

[jasnell]

nit: could these be moved into a table similar to the colors table at the top of the file?

[jasnell]

@redyetidev ... I'm +1 on getting it landed.

[avivkeller]

Hey, this feature modifies the console, and adds a new character that users may want to be aware of when logging certain character sequences. For that reason, I've applied the "notable change" label, but feel free to adjust.

[github-actions]

The notable-change PRs with changes that should be highlighted in changelogs. label has been added by @redyetidev.

Please suggest a text for the release notes if you'd like to include a more detailed summary, then proceed to update the PR description with the text or a link to the notable change suggested text comment. Otherwise, the commit will be placed in the Other Notable Changes section.

[github-actions]

This issue/PR was marked as stalled, it will be automatically closed in 30 days. If it should remain open, please leave a comment explaining why it should remain open.