Skip to content

util: add styleText API to text formatting - #51850

Merged
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
RafaelGSS:add-util-text-color
Feb 26, 2024
Merged

util: add styleText API to text formatting#51850
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
RafaelGSS:add-util-text-color

Conversation

@RafaelGSS

Copy link
Copy Markdown
Member

A fresh new PR from #43371.

@hemanth I took the liberty to create a new fresh PR (including a few changes) as your PR is almost 2 years old. I have included you as Co-Author I hope you don't mind (I tried to search for your handle in the OpenJS Foundation Slack, but I couldn't find it).

The concerns raised in the original blocker can be discussed again if necessary, but I believe we should have this API on Node.js.

cc: @nodejs/tsc

@RafaelGSSRafaelGSS added the semver-minor PRs that contain new features and should be released in the next minor version. label Feb 23, 2024
@nodejs-github-botnodejs-github-bot added needs-ci PRs that need a full CI run. util Issues and PRs related to the built-in util module. labels Feb 23, 2024

@mcollinamcollina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@mcollinamcollina added the request-ci Add this label to start a Jenkins CI on a PR. label Feb 23, 2024
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 23, 2024
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

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

@edsadredsadr left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really happy to see this happening 🎉

Comment threadlib/util.js Outdated
Comment threadtest/parallel/test-util-formattext.js Outdated
Comment threadtest/parallel/test-util-formattext.js Outdated
Comment threaddoc/api/util.md Outdated
Comment threaddoc/api/util.md Outdated
Comment threaddoc/api/util.md Outdated
Comment threaddoc/api/util.md Outdated
Comment threaddoc/api/util.md Outdated
@RafaelGSSRafaelGSS changed the title util: add formatText API to text formattingutil: add styleText API to text formattingFeb 23, 2024
@RafaelGSSRafaelGSS added the request-ci Add this label to start a Jenkins CI on a PR. label Feb 23, 2024
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 23, 2024
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

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

@aduh95aduh95 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider pushing an amend commit to update the commit message (git commit --allow-empty -m 'amend! util: add formatText API to text formatting' -m 'util: add `styleText` API for text formatting' -m 'Co-Authored-By: Hemanth HM <hemanth.hm@gmail.com>').

@RafaelGSSRafaelGSS added the request-ci Add this label to start a Jenkins CI on a PR. label Feb 23, 2024
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 23, 2024
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

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

Comment threaddoc/api/util.md Outdated
Comment threaddoc/api/util.md Outdated
Comment threadlib/util.js Outdated
@hemanth

Copy link
Copy Markdown
Contributor

Thanks a lot for taking the initiative to restart this effort, RafaelGSS! ❤️

Comment threaddoc/api/util.md Outdated
Co-Authored-By: Hemanth HM <hemanth.hm@gmail.com>
@RafaelGSSRafaelGSS added the request-ci Add this label to start a Jenkins CI on a PR. label Feb 25, 2024
marco-ippolito added a commit that referenced this pull request Mar 1, 2024
Notable changes:
build:
* (SEMVER-MINOR) build opt to set local location of headers (Michael Dawson) #51525
crypto:
* (SEMVER-MINOR) implement crypto.hash() (Joyee Cheung) #51044
* update root certificates to NSS 3.98 (Node.js GitHub Bot) #51794
doc:
* add zcbenz to collaborators (Cheng Zhao) #51812
* add lemire to collaborators (Daniel Lemire) #51572
http2:
* (SEMVER-MINOR) add h2 compat support for appendHeader (Tim Perry) #51412
* (SEMVER-MINOR) add server handshake utility (snek) #51172
* (SEMVER-MINOR) receive customsettings (Marten Richter) #51323
lib:
* (SEMVER-MINOR) move encodingsMap to internal/util (Joyee Cheung) #51044
sea:
* (SEMVER-MINOR) support sea.getRawAsset() (Joyee Cheung) #50960
* (SEMVER-MINOR) support embedding assets (Joyee Cheung) #50960
src:
* (SEMVER-MINOR) print string content better in BlobDeserializer (Joyee Cheung) #50960
* (SEMVER-MINOR) support multi-line values for .env file (IlyasShabi) #51289
* (SEMVER-MINOR) add `process.loadEnvFile` and `util.parseEnv` (Yagiz Nizipli) #51476
* (SEMVER-MINOR) do not coerce dotenv paths (Tobias Nießen) #51425
stream:
* (SEMVER-MINOR) implement `min` option for `ReadableStreamBYOBReader.read` (Mattias Buelens) #50888
util:
* (SEMVER-MINOR) add styleText API to text formatting (Rafael Gonzaga) #51850
vm:
* (SEMVER-MINOR) support using the default loader to handle dynamic import() (Joyee Cheung) #51244
PR-URL: #51932
marco-ippolito added a commit that referenced this pull request Mar 2, 2024
Notable changes:
build:
* (SEMVER-MINOR) build opt to set local location of headers (Michael Dawson) #51525
crypto:
* (SEMVER-MINOR) implement crypto.hash() (Joyee Cheung) #51044
* update root certificates to NSS 3.98 (Node.js GitHub Bot) #51794
doc:
* add zcbenz to collaborators (Cheng Zhao) #51812
* add lemire to collaborators (Daniel Lemire) #51572
http2:
* (SEMVER-MINOR) add h2 compat support for appendHeader (Tim Perry) #51412
* (SEMVER-MINOR) add server handshake utility (snek) #51172
* (SEMVER-MINOR) receive customsettings (Marten Richter) #51323
lib:
* (SEMVER-MINOR) move encodingsMap to internal/util (Joyee Cheung) #51044
sea:
* (SEMVER-MINOR) support sea.getRawAsset() (Joyee Cheung) #50960
* (SEMVER-MINOR) support embedding assets (Joyee Cheung) #50960
src:
* (SEMVER-MINOR) print string content better in BlobDeserializer (Joyee Cheung) #50960
* (SEMVER-MINOR) support multi-line values for .env file (IlyasShabi) #51289
* (SEMVER-MINOR) add `process.loadEnvFile` and `util.parseEnv` (Yagiz Nizipli) #51476
* (SEMVER-MINOR) do not coerce dotenv paths (Tobias Nießen) #51425
stream:
* (SEMVER-MINOR) implement `min` option for `ReadableStreamBYOBReader.read` (Mattias Buelens) #50888
util:
* (SEMVER-MINOR) add styleText API to text formatting (Rafael Gonzaga) #51850
vm:
* (SEMVER-MINOR) support using the default loader to handle dynamic import() (Joyee Cheung) #51244
PR-URL: #51932
marco-ippolito added a commit that referenced this pull request Mar 5, 2024
Notable changes:
build:
* (SEMVER-MINOR) build opt to set local location of headers (Michael Dawson) #51525
crypto:
* (SEMVER-MINOR) implement crypto.hash() (Joyee Cheung) #51044
* update root certificates to NSS 3.98 (Node.js GitHub Bot) #51794
doc:
* add zcbenz to collaborators (Cheng Zhao) #51812
* add lemire to collaborators (Daniel Lemire) #51572
http2:
* (SEMVER-MINOR) add h2 compat support for appendHeader (Tim Perry) #51412
* (SEMVER-MINOR) add server handshake utility (snek) #51172
* (SEMVER-MINOR) receive customsettings (Marten Richter) #51323
lib:
* (SEMVER-MINOR) move encodingsMap to internal/util (Joyee Cheung) #51044
sea:
* (SEMVER-MINOR) support sea.getRawAsset() (Joyee Cheung) #50960
* (SEMVER-MINOR) support embedding assets (Joyee Cheung) #50960
src:
* (SEMVER-MINOR) print string content better in BlobDeserializer (Joyee Cheung) #50960
* (SEMVER-MINOR) support multi-line values for .env file (IlyasShabi) #51289
* (SEMVER-MINOR) add `process.loadEnvFile` and `util.parseEnv` (Yagiz Nizipli) #51476
* (SEMVER-MINOR) do not coerce dotenv paths (Tobias Nießen) #51425
stream:
* (SEMVER-MINOR) implement `min` option for `ReadableStreamBYOBReader.read` (Mattias Buelens) #50888
util:
* (SEMVER-MINOR) add styleText API to text formatting (Rafael Gonzaga) #51850
vm:
* (SEMVER-MINOR) support using the default loader to handle dynamic import() (Joyee Cheung) #51244
PR-URL: #51932
marco-ippolito added a commit that referenced this pull request Mar 5, 2024
Notable changes:
build:
* (SEMVER-MINOR) build opt to set local location of headers (Michael Dawson) #51525
crypto:
* (SEMVER-MINOR) implement crypto.hash() (Joyee Cheung) #51044
* update root certificates to NSS 3.98 (Node.js GitHub Bot) #51794
doc:
* add zcbenz to collaborators (Cheng Zhao) #51812
* add lemire to collaborators (Daniel Lemire) #51572
http2:
* (SEMVER-MINOR) add h2 compat support for appendHeader (Tim Perry) #51412
* (SEMVER-MINOR) add server handshake utility (snek) #51172
* (SEMVER-MINOR) receive customsettings (Marten Richter) #51323
lib:
* (SEMVER-MINOR) move encodingsMap to internal/util (Joyee Cheung) #51044
sea:
* (SEMVER-MINOR) support sea.getRawAsset() (Joyee Cheung) #50960
* (SEMVER-MINOR) support embedding assets (Joyee Cheung) #50960
src:
* (SEMVER-MINOR) print string content better in BlobDeserializer (Joyee Cheung) #50960
* (SEMVER-MINOR) support multi-line values for .env file (IlyasShabi) #51289
* (SEMVER-MINOR) add `process.loadEnvFile` and `util.parseEnv` (Yagiz Nizipli) #51476
* (SEMVER-MINOR) do not coerce dotenv paths (Tobias Nießen) #51425
stream:
* (SEMVER-MINOR) implement `min` option for `ReadableStreamBYOBReader.read` (Mattias Buelens) #50888
util:
* (SEMVER-MINOR) add styleText API to text formatting (Rafael Gonzaga) #51850
vm:
* (SEMVER-MINOR) support using the default loader to handle dynamic import() (Joyee Cheung) #51244
PR-URL: #51932
RafaelGSS pushed a commit that referenced this pull request Mar 6, 2024
Notable changes:
build:
* (SEMVER-MINOR) build opt to set local location of headers (Michael Dawson) #51525
crypto:
* (SEMVER-MINOR) implement crypto.hash() (Joyee Cheung) #51044
* update root certificates to NSS 3.98 (Node.js GitHub Bot) #51794
doc:
* add zcbenz to collaborators (Cheng Zhao) #51812
* add lemire to collaborators (Daniel Lemire) #51572
http2:
* (SEMVER-MINOR) add h2 compat support for appendHeader (Tim Perry) #51412
* (SEMVER-MINOR) add server handshake utility (snek) #51172
* (SEMVER-MINOR) receive customsettings (Marten Richter) #51323
lib:
* (SEMVER-MINOR) move encodingsMap to internal/util (Joyee Cheung) #51044
sea:
* (SEMVER-MINOR) support sea.getRawAsset() (Joyee Cheung) #50960
* (SEMVER-MINOR) support embedding assets (Joyee Cheung) #50960
src:
* (SEMVER-MINOR) print string content better in BlobDeserializer (Joyee Cheung) #50960
* (SEMVER-MINOR) support multi-line values for .env file (IlyasShabi) #51289
* (SEMVER-MINOR) add `process.loadEnvFile` and `util.parseEnv` (Yagiz Nizipli) #51476
* (SEMVER-MINOR) do not coerce dotenv paths (Tobias Nießen) #51425
stream:
* (SEMVER-MINOR) implement `min` option for `ReadableStreamBYOBReader.read` (Mattias Buelens) #50888
util:
* (SEMVER-MINOR) add styleText API to text formatting (Rafael Gonzaga) #51850
vm:
* (SEMVER-MINOR) support using the default loader to handle dynamic import() (Joyee Cheung) #51244
PR-URL: #51932
crowlKats added a commit to denoland/deno that referenced this pull request Mar 6, 2024
Comment threadlib/util.js
*/
function styleText(format, text) {
validateString(text, 'text');
const formatCodes = inspect.colors[format];

@kibertoadkibertoadMar 6, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does it make sense to do this check on every call? could be done once and cached

nvm, happy path should be pretty fast here, misread the code

@hemanth

Copy link
Copy Markdown
Contributor

I was wondering if there have been any updates on this issue. It appears that #43371 was marked as blocked pending a decision. \cc @mcollina

@mcollina

Copy link
Copy Markdown
Member

I guess this module landing was a decision on that.

@bakkot

bakkot commented Mar 7, 2024

Copy link
Copy Markdown
Contributor

Seems kind of a shame to have left out support for the common NO_COLOR option environment variable, or for checking isatty and FORCE_COLOR. That means people trying to switch to this from (e.g.) picocolors or chalk or colors.js or ansi-colors or colorette or kleur are (probably unknowingly) going to end up with a worse experience for their users. Node itself supports those as well.

I'm very much in favor of the effort to pull more widely-used features into core, but I feel like the process ought to include a bit more review of what conventions the ecosystem has settled on before doing so.

@meyfa

meyfa commented Mar 7, 2024

Copy link
Copy Markdown
Contributor

This is at stability 1.1 (Active Development), is it not? Wouldn't that mean that such functionality can still be added?

@kibertoad

Copy link
Copy Markdown
Contributor

It only landed in 21.x so far, so yeah, should be OK to change it

@RafaelGSS

Copy link
Copy Markdown
MemberAuthor

Folks, please see #51959. Consider opening a new issue to discuss the feature and stability of this new API instead of commenting on this PR.

littledivy pushed a commit to denoland/deno that referenced this pull request Mar 8, 2024
hail2u added a commit to hail2u/hail2u.net that referenced this pull request Mar 12, 2024
richardlau pushed a commit that referenced this pull request Mar 25, 2024
Co-Authored-By: Hemanth HM <hemanth.hm@gmail.com>
PR-URL: #51850
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Adrian Estrada <edsadr@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
richardlau added a commit that referenced this pull request Mar 25, 2024
Notable changes:
build:
* (SEMVER-MINOR) build opt to set local location of headers (Michael Dawson) #51525
crypto:
* (SEMVER-MINOR) implement crypto.hash() (Joyee Cheung) #51044
* update root certificates to NSS 3.98 (Node.js GitHub Bot) #51794
doc:
* add lemire to collaborators (Daniel Lemire) #51572
* add zcbenz to collaborators (Cheng Zhao) #51812
lib:
* (SEMVER-MINOR) move encodingsMap to internal/util (Joyee Cheung) #51044
sea:
* (SEMVER-MINOR) support sea.getRawAsset() (Joyee Cheung) #50960
* (SEMVER-MINOR) support embedding assets (Joyee Cheung) #50960
src:
* (SEMVER-MINOR) print string content better in BlobDeserializer (Joyee Cheung) #50960
util:
* (SEMVER-MINOR) add styleText API to text formatting (Rafael Gonzaga) #51850
vm:
* (SEMVER-MINOR) support using the default loader to handle dynamic import() (Joyee Cheung) #51244
PR-URL: #52212
@richardlaurichardlau mentioned this pull request Mar 25, 2024
richardlau added a commit that referenced this pull request Mar 26, 2024
Notable changes:
build:
* (SEMVER-MINOR) build opt to set local location of headers (Michael Dawson) #51525
crypto:
* (SEMVER-MINOR) implement crypto.hash() (Joyee Cheung) #51044
* update root certificates to NSS 3.98 (Node.js GitHub Bot) #51794
doc:
* add lemire to collaborators (Daniel Lemire) #51572
* add zcbenz to collaborators (Cheng Zhao) #51812
lib:
* (SEMVER-MINOR) move encodingsMap to internal/util (Joyee Cheung) #51044
sea:
* (SEMVER-MINOR) support sea.getRawAsset() (Joyee Cheung) #50960
* (SEMVER-MINOR) support embedding assets (Joyee Cheung) #50960
src:
* (SEMVER-MINOR) print string content better in BlobDeserializer (Joyee Cheung) #50960
util:
* (SEMVER-MINOR) add styleText API to text formatting (Rafael Gonzaga) #51850
vm:
* (SEMVER-MINOR) support using the default loader to handle dynamic import() (Joyee Cheung) #51244
PR-URL: #52212
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.needs-ciPRs that need a full CI run.notable-changePRs with changes that should be highlighted in changelogs.semver-minorPRs that contain new features and should be released in the next minor version.utilIssues and PRs related to the built-in util module.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

12 participants

@RafaelGSS@nodejs-github-bot@hemanth@mcollina@bakkot@meyfa@kibertoad@GeoffreyBooth@edsadr@MoLow@aduh95@marco-ippolito