util: add colorText method by hemanth · Pull Request #43371 · nodejs/node

hemanth · 2022-06-10T16:54:55Z

This is with reference to the comment in #42770

A few things to finalize before completing this draft PR:

Are we Ok to have such a method?
Given that is it CLI only the name sounds fine?
Can it be part of the console API?

//cc @nodejs/util

test/parallel/test-util-colorText.js

lib/util.js

ljharb · 2022-06-10T18:03:51Z

Name sounds good to me; i don't think it should be in console at all.

Is there any way to have tests that actually verify that the expected colors show up on supported platforms?

cjihrig

Left some preliminary comments.

Is there any way to have tests that actually verify that the expected colors show up on supported platforms?

test/parallel/test-util-format.js and test/parallel/test-util-inspect.js might be good places to look for examples.

Also cc @BridgeAR who might be interested since this overlaps with the color support in util.inspect().

lib/util.js

cjihrig · 2022-06-11T18:32:53Z

lib/util.js

+ function colorText(format, text) {
+  validateString(format, 'format');
+  validateString(text, 'text');
+  const formatCodes = inspect.colors[format];


If this is going to be public API, we should probably provide some additional validation for supported formats.

additional validation

like?

I was thinking we could validate that format is actually something supported by inspect.colors, but feel free to ignore if you want.

If they pass anything apart as of now, we are just returning the text as in, do you feel it makes sense to check if the format is supported by inspect.colors and throw an error for non-supported inputs?

Yes, that's what I originally meant.

Is inspect.colors mutable and exposed to users?

lib/util.js

test/parallel/test-util-colorText.js

Trott · 2022-06-13T21:21:00Z

Micro-nit on the commit message: The verb in the first line of the commit message should be imperative. In other words, add rather than adding. This is something that someone can fix when landing, and also no one (not even me) is actually going to care if it slips through as adding rather than add. But if you want to make that change, that would make it comply with our contributor guidelines doc.

hemanth · 2022-06-13T22:11:23Z

Thanks for the clarification @Trott. I had that doubt, fixed it.

nodejs-github-bot · 2022-06-14T19:51:37Z

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

hemanth · 2022-06-14T22:30:56Z

We should also add this to the docs right?

cjihrig · 2022-06-14T23:09:57Z

Yes, this would require a docs update.

hemanth · 2022-06-17T22:31:17Z

Is this ready enough?

cjihrig

Left a few comments. It would be good for others to weigh in on things like the overall API design and whether or not we should make this experimental first.

doc/api/util.md

cjihrig · 2022-06-21T02:21:01Z

doc/api/util.md

+added: v18.3.0
+-->
+
+* `format` {string} `format` one of the color format from `util.inspect.colors`


We should probably link util.inspect.colors to https://nodejs.org/api/util.html#customizing-utilinspect-colors.

doc/api/util.md

cjihrig · 2022-06-21T02:29:23Z

doc/api/util.md

+Takes `format` and `text` and retuns the colored text form
+
+```js
+const util = require('node:util');


Can you remove this line. Otherwise we'll need to show the same example code for CJS and ESM.

cjihrig · 2022-06-21T02:29:58Z

doc/api/util.md

+const util = require('node:util');
+
+console.log(util.colorText('red', 'This text shall be in red color'));
+// ^ '\u001b[31mThis text shall be in red color\u001b[39m'


Suggested change

// ^ '\u001b[31mThis text shall be in red color\u001b[39m'

// ^ '\u001b[31mThis text shall be colored red\u001b[39m'

s/shall be/is?

cjihrig · 2022-06-21T02:30:56Z

lib/util.js

+ function colorText(format, text) {
+  validateString(format, 'format');
+  validateString(text, 'text');
+  const formatCodes = inspect.colors[format];


Yes, that's what I originally meant.

cjihrig · 2022-06-21T02:33:08Z

test/parallel/test-util-colorText.js

+assert.throws(() => {
+  util.colorText('red', undefined);
+}, {
+  code: 'ERR_INVALID_ARG_TYPE',
+  message: 'The "text" argument must be of type string. Received undefined'
+});


Wouldn't this already be tested on line 20 when invalidOption is undefined?

Right, this should rather be util.colorText(undefined, undefined);

doc/api/util.md

ljharb · 2022-06-21T04:15:50Z

doc/api/util.md

+const util = require('node:util');
+
+console.log(util.colorText('red', 'This text shall be in red color'));
+// ^ '\u001b[31mThis text shall be in red color\u001b[39m'


s/shall be/is?

ljharb · 2022-06-21T04:16:10Z

lib/util.js

+ function colorText(format, text) {
+  validateString(format, 'format');
+  validateString(text, 'text');
+  const formatCodes = inspect.colors[format];


Is inspect.colors mutable and exposed to users?

Co-authored-by: Colin Ihrig <cjihrig@gmail.com>

Co-authored-by: Jordan Harband <ljharb@gmail.com>

BridgeAR

I am hesitant to add new formatting functionalities to util (console does not seem ideal either). I just opened a something similar API: #43523. It does a bit more by also allowing nested colors, this functionality here does not do that (e.g., colorText('green', 'green ' + colorText('red', 'red') + ' green') would end up being colored: green red defaultColor instead of green red green).

jasnell · 2022-06-27T17:18:13Z

Let's throw this into the TSC agenda as part of the discussion for #43523

mcollina · 2022-07-13T12:23:50Z

Dropping it from the tsc-agenda until #43382 is resolved.

felixfbecker · 2024-03-06T21:52:06Z

doc/api/util.md

+added: REPLACEME
+-->
+
+* `format` {string} A color format defined in `util.inspect.colors`.


Accepting string | string[] would be really helpful to e.g. make text both a certain color and underline/italic/bold/... at the same time

mcollina · 2024-03-07T09:11:32Z

Superseded by #51850

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

hemanth commented Jun 10, 2022

View reviewed changes

test/parallel/test-util-colorText.js Outdated Show resolved Hide resolved

hemanth force-pushed the util-format-text branch from 26f5680 to 7556c9e Compare June 10, 2022 16:58

aduh95 reviewed Jun 10, 2022

View reviewed changes

lib/util.js Outdated Show resolved Hide resolved

cjihrig reviewed Jun 11, 2022

View reviewed changes

hemanth changed the title ~~WIP: Adding colorText method to util.~~ util: [WIP] Adding colorText method to util. Jun 11, 2022

hemanth force-pushed the util-format-text branch 3 times, most recently from 9c94ffe to 9ac1a84 Compare June 12, 2022 06:03

hemanth changed the title ~~util: [WIP] Adding colorText method to util.~~ util: Adding colorText method to util Jun 13, 2022

util: add colorText method

7cd400f

hemanth force-pushed the util-format-text branch from 9ac1a84 to 7cd400f Compare June 13, 2022 22:11

Trott added the request-ci Add this label to start a Jenkins CI on a PR. label Jun 14, 2022

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 14, 2022

hemanth force-pushed the util-format-text branch from 01a77c8 to ccfdcee Compare June 14, 2022 23:32

doc: add colorText

9da85fe

hemanth force-pushed the util-format-text branch from ccfdcee to 9da85fe Compare June 14, 2022 23:45

This was referenced Jun 15, 2022

CI Reliability 2022-06-15 nodejs/reliability#301

Open

CI Reliability 2022-06-16 nodejs/reliability#302

Open

CI Reliability 2022-06-17 nodejs/reliability#303

Open

hemanth changed the title ~~util: Adding colorText method to util~~ util: add colorText method Jun 20, 2022

hemanth requested a review from cjihrig June 20, 2022 15:48

hemanth requested a review from aduh95 June 20, 2022 15:48

cjihrig reviewed Jun 21, 2022

View reviewed changes

ljharb reviewed Jun 21, 2022

View reviewed changes

hemanth and others added 5 commits June 20, 2022 21:52

Update doc/api/util.md

baad390

Co-authored-by: Colin Ihrig <cjihrig@gmail.com>

Update doc/api/util.md

f4814d4

Co-authored-by: Colin Ihrig <cjihrig@gmail.com>

Update doc/api/util.md

d21bd40

Co-authored-by: Colin Ihrig <cjihrig@gmail.com>

Update doc/api/util.md

d48eb4f

Co-authored-by: Colin Ihrig <cjihrig@gmail.com>

Update doc/api/util.md

3bcbc5d

Co-authored-by: Jordan Harband <ljharb@gmail.com>

BridgeAR reviewed Jun 21, 2022

View reviewed changes

jasnell added the tsc-agenda Issues and PRs to discuss during the meetings of the TSC. label Jun 27, 2022

This was referenced Jul 4, 2022

Node.js Technical Steering Committee (TSC) Meeting 2022-07-06 nodejs/TSC#1253

Closed

Node.js Technical Steering Committee (TSC) Meeting 2022-07-13 nodejs/TSC#1259

Closed

mcollina removed the tsc-agenda Issues and PRs to discuss during the meetings of the TSC. label Jul 13, 2022

mcollina added the blocked PRs that are blocked by other issues or PRs. label Jul 13, 2022

Trott force-pushed the main branch from 2d76238 to ca3ed36 Compare November 12, 2022 01:49

RafaelGSS mentioned this pull request Feb 23, 2024

util: add styleText API to text formatting #51850

Merged

felixfbecker reviewed Mar 6, 2024

View reviewed changes

mcollina closed this Mar 7, 2024

	// ^ '\u001b[31mThis text shall be in red color\u001b[39m'
	// ^ '\u001b[31mThis text shall be colored red\u001b[39m'

Uh oh!

Conversation

hemanth commented Jun 10, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ljharb commented Jun 10, 2022

Uh oh!

cjihrig left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hemanth Jun 11, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Trott commented Jun 13, 2022

Uh oh!

hemanth commented Jun 13, 2022

Uh oh!

nodejs-github-bot commented Jun 14, 2022

Uh oh!

hemanth commented Jun 14, 2022

Uh oh!

cjihrig commented Jun 14, 2022

Uh oh!

hemanth commented Jun 17, 2022

Uh oh!

cjihrig left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BridgeAR left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jasnell commented Jun 27, 2022

Uh oh!

mcollina commented Jul 13, 2022

Uh oh!

Choose a reason for hiding this comment

hemanth commented Jun 10, 2022 •

edited

Loading

hemanth Jun 11, 2022 •

edited

Loading

BridgeAR left a comment •

edited

Loading