-
Notifications
You must be signed in to change notification settings - Fork 30k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
console: colorize console error and warn #51629
console: colorize console error and warn #51629
Conversation
Will add test cases if this feature is acceptable |
827fe0c
to
b69e24d
Compare
0a37526
to
3b4f498
Compare
@MoLow I have changed the value of |
3b4f498
to
d5ac03b
Compare
I believe it's important to run through citgm to gauge the impact of this change. |
d5ac03b
to
3bf4dbf
Compare
3bf4dbf
to
d07b7b4
Compare
I once again believe this should be marked as semver major change: #51503 |
This comment was marked as outdated.
This comment was marked as outdated.
471a7d4
to
8747c98
Compare
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
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 |
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>
Landed in a833c9e |
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>
Updated node to latest version, but console.error still outputs unclolored text. Is there any configuration required to achieve this? |
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>
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>
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>
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>
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 I would rather that this PR implemented color as a opt in vs trying to figure out how to opt out. |
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
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
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>
Related to : 40361
Colorise string type args of
console.error
andconsole.warn
. We are skipping the non-string types.Tasks: