FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

console: colorize console error and warn by MrJithil · Pull Request #51629 · nodejs/node · GitHub

/ node Public

console: colorize console error and warn - #51629

Closed
MrJithil wants to merge 10 commits into
nodejs:mainfrom
MrJithil:colorize-console-error-and-warning
Closed

console: colorize console error and warn#51629
MrJithil wants to merge 10 commits into
nodejs:mainfrom
MrJithil:colorize-console-error-and-warning

Conversation

MrJithil commented Feb 1, 2024
edited
Loading

Copy link
Copy Markdown
Member

Related to : 40361

Colorise string type args of console.error and console.warn. We are skipping the non-string types.

Tasks:

  • Run CITGM

nodejs-github-bot added console Issues and PRs related to the console subsystem. needs-ci PRs that need a full CI run. labels Feb 1, 2024

MrJithil commented Feb 1, 2024

Copy link
Copy Markdown
Member Author

Will add test cases if this feature is acceptable

MrJithil force-pushed the colorize-console-error-and-warning branch 5 times, most recently from 827fe0c to b69e24d Compare February 1, 2024 13:22
Comment thread lib/internal/console/constructor.js Outdated
MrJithil force-pushed the colorize-console-error-and-warning branch 8 times, most recently from 0a37526 to 3b4f498 Compare February 5, 2024 11:30

MrJithil commented Feb 5, 2024

Copy link
Copy Markdown
Member Author

@MoLow I have changed the value of clear from '\u001bc' to '\u001b[0m'. Please check.

MrJithil force-pushed the colorize-console-error-and-warning branch from 3b4f498 to d5ac03b Compare February 5, 2024 11:39
ruyadorno added the needs-citgm PRs that need a CITGM CI run. label Feb 5, 2024

ruyadorno commented Feb 5, 2024
edited
Loading

Copy link
Copy Markdown
Member

I believe it's important to run through citgm to gauge the impact of this change.

MrJithil force-pushed the colorize-console-error-and-warning branch from d5ac03b to 3bf4dbf Compare February 6, 2024 11:26
MrJithil force-pushed the colorize-console-error-and-warning branch from 3bf4dbf to d07b7b4 Compare March 14, 2024 09:32

Copy link
Copy Markdown
Member

I once again believe this should be marked as semver major change: #51503

marco-ippolito added the blocked PRs that are blocked by other issues or PRs. label Mar 15, 2024

This comment was marked as outdated.

MrJithil force-pushed the colorize-console-error-and-warning branch 2 times, most recently from 471a7d4 to 8747c98 Compare March 20, 2024 10:09
MrJithil requested a review from MoLow March 20, 2024 10:10

This comment was marked as outdated.

This comment was marked as outdated.

Copy link
Copy Markdown
Collaborator

Copy link
Copy Markdown
Member Author

CITGM: https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/3422

Could you please help me to run this CIGTM? I haven't run this before and felt a little confusing. Since these tests are time consuming, thought to take help or opinion before retrying it.

MrJithil added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Apr 30, 2024

aduh95 commented May 5, 2024

Copy link
Copy Markdown
Contributor

CITGM: https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/3422

Could you please help me to run this CIGTM? I haven't run this before and felt a little confusing. Since these tests are time consuming, thought to take help or opinion before retrying it.

$ ncu-ci.js citgm 3421 3422

FAILURE: 6 failures in 3422 not present in 3421


┌────────────────────────┬──────────────────┬───────────────────────┐
│        (index)         │        0         │           1           │
├────────────────────────┼──────────────────┼───────────────────────┤
│       osx11-x64        │                  │                       │
│      rhel8-s390x       │                  │                       │
│       win-vs2022       │                  │                       │
│ alpine-last-latest-x64 │                  │                       │
│       rhel8-x64        │ 'undici-v6.14.1' │                       │
│      debian12-x64      │                  │                       │
│      debian11-x64      │  'jest-v0.0.0'   │    'semver-v7.6.0'    │
│   fedora-latest-x64    │                  │                       │
│     rhel8-ppc64le      │  'pino-v8.19.0'  │ 'prom-client-v15.1.2' │
│      aix72-ppc64       │                  │                       │
│     ubuntu2204-64      │ 'undici-v6.14.1' │                       │
│   alpine-latest-x64    │                  │                       │
│ fedora-last-latest-x64 │                  │                       │
└────────────────────────┴──────────────────┴───────────────────────┘

Now it's a matter of investigating whether those failures were caused by this PR or something else. For semver and prom-client, that's easy, No space left of device, so unrelated. For the other ones, it would require more careful review – but it seems fair to say landing this PR would not cause to much trouble to the ecosystem.

MrJithil added a commit that referenced this pull request May 8, 2024
prints console error in red and warn in yellow

PR-URL: #51629
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Ruy Adorno <ruy@vlt.sh>
Reviewed-By: James M Snell <jasnell@gmail.com>

MrJithil commented May 8, 2024

Copy link
Copy Markdown
Member Author

Landed in a833c9e

MrJithil closed this May 8, 2024
MrJithil deleted the colorize-console-error-and-warning branch May 8, 2024 13:59
targos pushed a commit that referenced this pull request May 11, 2024
prints console error in red and warn in yellow

PR-URL: #51629
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Ruy Adorno <ruy@vlt.sh>
Reviewed-By: James M Snell <jasnell@gmail.com>
ruyadorno removed the blocked PRs that are blocked by other issues or PRs. label May 13, 2024

Copy link
Copy Markdown

Updated node to latest version, but console.error still outputs unclolored text. Is there any configuration required to achieve this?

marco-ippolito pushed a commit that referenced this pull request Jun 17, 2024
prints console error in red and warn in yellow

PR-URL: #51629
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Ruy Adorno <ruy@vlt.sh>
Reviewed-By: James M Snell <jasnell@gmail.com>
soophoo pushed a commit to soophoo/node that referenced this pull request Jun 20, 2024
prints console error in red and warn in yellow

PR-URL: nodejs#51629
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Ruy Adorno <ruy@vlt.sh>
Reviewed-By: James M Snell <jasnell@gmail.com>

Jason3S commented Jun 29, 2024
edited
Loading

Copy link
Copy Markdown

I'm a bit surprised that this landed without a link to the discussion. #40361 is a feature request, but doesn't have a real discussion around the pros/cons and how it will impact the nodejs ecosystem.

I would have rather seen color added to the formatting of Errors instead of blanket red/yellow for console.error/console.warn.

I realize that in some way, this PR makes the behavior closer to that in the browser, but browsers do not have CLI applications.

I treat console.log and console.error as separate channels. Traditionally stdout is used for output of the program. It contains the result to be used, think grep. stderr is used for diagnostics, status, and messages to the user while the application is running. Color is not implied.

I would rather that this PR implemented color as a opt in vs trying to figure out how to opt out.

convex-copybara Bot pushed a commit to get-convex/convex-backend that referenced this pull request Jul 13, 2024
Recent versions of Node.js [print console.error() in red](nodejs/node#51629), changing the color of convex CLI output unintentionally. Change everywhere we `console.error` as a means to write to stderr to `process.stderr.write` calls.

GitOrigin-RevId: 134e5302858c309e3a7fd9d470a685d9bf60b1df
convex-copybara Bot pushed a commit to get-convex/convex-js that referenced this pull request Jul 13, 2024
Recent versions of Node.js [print console.error() in red](nodejs/node#51629), changing the color of convex CLI output unintentionally. Change everywhere we `console.error` as a means to write to stderr to `process.stderr.write` calls.

GitOrigin-RevId: 134e5302858c309e3a7fd9d470a685d9bf60b1df
kevinoid added a commit to kevinoid/noderegression that referenced this pull request Nov 2, 2024
The output of console.Console is colorized in Node.js >=20.15 <22.10
due to nodejs/node#51629 and
nodejs/node#54677.  Handle this in our test
cases by comparing the output to the output of console.Console on a
PassThrough stream.

Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author ready PRs that have at least one approval, no outstanding review comments, and a CI started. console Issues and PRs related to the console subsystem. needs-ci PRs that need a full CI run. needs-citgm PRs that need a CITGM CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants


Back | FazBrowse Home | New Git URL