util: add colorText method#43371

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

+functioncolorText(format,text){
+validateString(format,'format');
+validateString(text,'text');
+constformatCodes=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
+constutil=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

+constutil=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

+functioncolorText(format,text){
+validateString(format,'format');
+validateString(text,'text');
+constformatCodes=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

+constutil=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

+functioncolorText(format,text){
+validateString(format,'format');
+validateString(text,'text');
+constformatCodes=inspect.colors[format];


Is inspect.colors mutable and exposed to users?

Co-authored-by: Colin Ihrig <[email protected]>

Co-authored-by: Jordan Harband <[email protected]>

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 resolvedHide resolved

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

aduh95 reviewed Jun 10, 2022
View reviewed changes

lib/util.js Outdated Show resolvedHide 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 9ac1a84Compare June 12, 2022 06:03

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

util: add colorText method
7cd400f

hemanth force-pushed the util-format-text branch from 9ac1a84 to 7cd400fCompare 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-actionsbot 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 ccfdceeCompare June 14, 2022 23:32

doc: add colorText
9da85fe

hemanth force-pushed the util-format-text branch from ccfdcee to 9da85feCompare 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 methodJun 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

hemanthand others added 5 commits June 20, 2022 21:52

Update doc/api/util.md
baad390
Co-authored-by: Colin Ihrig <[email protected]>

Update doc/api/util.md
f4814d4
Co-authored-by: Colin Ihrig <[email protected]>

Update doc/api/util.md
d21bd40
Co-authored-by: Colin Ihrig <[email protected]>

Update doc/api/util.md
d48eb4f
Co-authored-by: Colin Ihrig <[email protected]>

Update doc/api/util.md
3bcbc5d
Co-authored-by: Jordan Harband <[email protected]>

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 ca3ed36Compare 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!

util: add colorText method#43371

util: add colorText method #43371

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!

hemanthJun 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!

hemanth commented Jun 10, 2022•
edited
Loading

hemanthJun 11, 2022•
edited
Loading

BridgeAR left a comment •
edited
Loading