util: add colorize functionality #43523

BridgeAR · 2022-06-21T15:44:02Z

The colorize object allows to format strings with ansi color codes
so that the string is stylized on a terminal.

This is a utility function that should not yet be exposed to the
users to allow some further polishing first.
This should be exposed under a completely new formatter module instead of adding new functionality to util.
Adding this functionality was discussed at the collaborator summit.

I played around with different builder APIs such as:

new Colorize().bold.green('bold green')
colorize().bold().green('bold green')
colorize.bold.green('bold green')

In the end I went with the latter, especially, due to chalk already using this.

Refs: #43382

Signed-off-by: Ruben Bridgewater ruben@bridgewater.de

@nodejs/util @nodejs/tooling PTAL.

The colorize object allows to format strings with ansi color codes so that the string is stylized on a terminal. This is a utility function that should not yet be exposed to the users to allow some further polishing first. Signed-off-by: Ruben Bridgewater <ruben@bridgewater.de>

ljharb · 2022-06-21T16:35:55Z

doc/api/util.md

+)
+
+console.log(
+  colorize.bold.underline.red('bold red underline')


i would definitely prefer to avoid getters; both because it's a more confusing API, and because it's slower. i'd follow colors here, not chalk.

Colors supports this notation? Manipulating the string prototype is not ideal (that's the original colors API).

oh no, i meant colors/safe :-) definitely nobody should use the colors style :-)

I am somewhat puzzled: that exact API is the one implemented here?

oh, maybe i'm confused. i'd prefer colorize.bold().underline() to colorize.bold.underline, to be clearer.

I did consider that as well and I am open to using that, if it's what a majority prefers. Right now I would stick to the existing due to these reasons:

Users of colors and chalk are used to it

Passing a string to only the last call might also be confusing without a .write()/.format() similar function

The performance should pretty much be identical and there should be room for improvement one way or the other (the implementation is already decent performance wise while chalk is still faster)

Qix- · 2022-06-21T18:43:25Z

Can I ask, why are we moving established package functionality from the ecosystem into core? What is the goal by providing this as a standard module?

ljharb · 2022-06-21T18:47:15Z

@Qix- in this particular case, the established packages are not reliable. One has gone ESM-only, and the other was turned into malware by its maintainer.

This is something core should have provided many years ago, but for these packages, and it's now critical it does so, for security and reliability reasons.

GeoffreyBooth · 2022-06-21T22:10:39Z

doc/api/util.md

+added: REPLACEME
+-->
+
+The `util.colorize` object provides functions that add ansi color codes to the


Suggested change

The `util.colorize` object provides functions that add ansi color codes to the

The `util.colorize` object provides functions that add ANSI color codes to the

A citation for “ANSI color codes” would be a good addition.

GeoffreyBooth · 2022-06-21T22:13:02Z

lib/internal/util/inspect.js

+// TODO(BridgeAR): Consider using a class instead and require the user to instantiate it
+// before using to declare their color support (or use a default, if none
+// provided).


Why wouldn’t or couldn’t these be simple static functions that require no initialization? I mean, aren’t they essentially just prepending and appending \u001b[90m and \u001b[39m and the like?

As a user, I’d prefer they be as terse as possible. I’d be more likely to use them the simpler they are.

Yes and no: if we add more colors such as hex colors later on, we'll need to check that the terminal does indeed support the colors. Currently there's no way to do this besides a best effort check and we sometimes detect colors when not being supported and the other way around. Therefore I would like to provide a way to define the support by the user:

const { Colorize } = require('formatter') const colorize256 = new Colorize({ colors: 256 }) console.log( colorize256.bold.underline.hex('#123456')('bold underline fancy color') )

Using Colorize.bold.underline.hex('#123456')('bold underline maybe fancy color') could try to auto detect the supported colors while using the instance would be fixed.

These feel like almost two use cases: a simple CLI tool like a test runner that needs only a handful of colors (or only green and red); and a more complicated CLI tool that wants lots of colors. The latter can call some kind of initialization function, since it’s somewhat expected that more work would be required to do something fancy; but ideally the simple case wouldn’t need much ceremony.

GeoffreyBooth · 2022-06-21T22:13:56Z

lib/internal/util/inspect.js

+  });
+}
+
+for (const [style, descriptor] of Object.entries(Object.getOwnPropertyDescriptors(inspect.colors))) {


Do we need to use primordials here? @aduh95

it's not as important at the top level, but it'd probably be a good idea for consistency

Qix- · 2022-06-22T00:46:15Z

One has gone ESM-only, and the other was turned into malware by its maintainer.

What do you expect us to do when the TC-39 introduces fundamental language changes without any migration plan? We put off porting for literal years, and when it became more of a problem not to, now we're being told we're being hostile?

Is Node just going to integrate every popular npm package at once?

GeoffreyBooth · 2022-06-22T01:48:31Z

What do you expect us to do

Let's please keep the discussion in this thread focused on the technical details of this PR. If you'd like to debate the feature itself, please open a discussion issue. Product debate on PR threads makes PRs difficult to review.

F3n67u · 2022-06-22T02:31:57Z

I think if there is any question about adding this feature, we could move to #43382 which @BridgeAR created for this feature.

mcollina

A few notes:

what happens if colors are not available in the TTY?
how could somebody override that setting?

I think we should provide a constructor too where this could be configured.

benjamingr

#43382 (comment) want to wait for this to conclude

BridgeAR · 2022-06-25T12:41:49Z

I am closing this for now until we are able to resolve the discussion in #43382

nodejs-github-bot added debugger Issues and PRs related to the debugger subsystem. needs-ci PRs that need a full CI run. repl Issues and PRs related to the REPL subsystem. util Issues and PRs related to the built-in util module. labels Jun 21, 2022

BridgeAR force-pushed the add-color-helpers branch from 6da45e0 to 77f2290 Compare June 21, 2022 16:00

ljharb reviewed Jun 21, 2022

View reviewed changes

GeoffreyBooth reviewed Jun 21, 2022

View reviewed changes

mcollina requested changes Jun 22, 2022

View reviewed changes

benjamingr requested changes Jun 23, 2022

View reviewed changes

Qix- mentioned this pull request Jun 25, 2022

Consider explicit string formatting module #43382

Closed

BridgeAR closed this Jun 25, 2022

jasnell mentioned this pull request Jun 27, 2022

util: add colorText method #43371

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

util: add colorize functionality #43523

util: add colorize functionality #43523

BridgeAR commented Jun 21, 2022 •

edited by VoltrexKeyva

Loading

ljharb Jun 21, 2022

BridgeAR Jun 21, 2022

ljharb Jun 21, 2022

BridgeAR Jun 21, 2022

ljharb Jun 21, 2022

BridgeAR Jun 21, 2022

Qix- commented Jun 21, 2022

ljharb commented Jun 21, 2022

GeoffreyBooth Jun 21, 2022

GeoffreyBooth Jun 21, 2022

BridgeAR Jun 21, 2022

GeoffreyBooth Jun 21, 2022

GeoffreyBooth Jun 21, 2022

ljharb Jun 21, 2022

Qix- commented Jun 22, 2022

GeoffreyBooth commented Jun 22, 2022

F3n67u commented Jun 22, 2022 •

edited

Loading

mcollina left a comment •

edited

Loading

benjamingr left a comment

BridgeAR commented Jun 25, 2022

	The `util.colorize` object provides functions that add ansi color codes to the
	The `util.colorize` object provides functions that add ANSI color codes to the

util: add colorize functionality #43523

util: add colorize functionality #43523

Conversation

BridgeAR commented Jun 21, 2022 • edited by VoltrexKeyva Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Qix- commented Jun 21, 2022

ljharb commented Jun 21, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Qix- commented Jun 22, 2022

GeoffreyBooth commented Jun 22, 2022

F3n67u commented Jun 22, 2022 • edited Loading

mcollina left a comment • edited Loading

Choose a reason for hiding this comment

benjamingr left a comment

Choose a reason for hiding this comment

BridgeAR commented Jun 25, 2022

BridgeAR commented Jun 21, 2022 •

edited by VoltrexKeyva

Loading

F3n67u commented Jun 22, 2022 •

edited

Loading

mcollina left a comment •

edited

Loading