Skip to content

repl: fix NO_COLORS env var is ignored - #51568

Merged
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
MoLow:fix-no-colors
Jan 31, 2024
Merged

repl: fix NO_COLORS env var is ignored#51568
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
MoLow:fix-no-colors

Conversation

@MoLow

@MoLowMoLow commented Jan 26, 2024

Copy link
Copy Markdown
Member

refs: #48034 (comment)

the logic in options.useColors = shouldColorize(options.output) || process.env.NODE_DISABLE_COLORS === undefined; indroduced this bug since shouldColorize already checks NODE_DISABLE_COLORS among other env vars that disable colors: NO_COLOR and TERM === 'dumb'

@MoLow
MoLow requested a review from addaleaxJanuary 26, 2024 08:39
@nodejs-github-botnodejs-github-bot added needs-ci PRs that need a full CI run. repl Issues and PRs related to the REPL subsystem. labels Jan 26, 2024
BridgeAR
BridgeAR approved these changes Jan 26, 2024
View reviewed changes
Comment threadtest/parallel/test-repl-options.js
@MoLowMoLow added the request-ci Add this label to start a Jenkins CI on a PR. label Jan 26, 2024
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 26, 2024
@nodejs-github-bot

nodejs-github-bot commented Jan 26, 2024

Copy link
Copy Markdown
Collaborator

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

addaleax
addaleax approved these changes Jan 26, 2024
View reviewed changes

@addaleaxaddaleax 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.

Thank you!

@anonrig

anonrig commented Jan 27, 2024

Copy link
Copy Markdown
Member

Can we add a test, so we don't regress?

@MoLow

MoLow commented Jan 27, 2024

Copy link
Copy Markdown
MemberAuthor

Can we add a test, so we don't regress?

there were already existing tests that needed fixing

@nodejs-github-bot

nodejs-github-bot commented Jan 27, 2024

Copy link
Copy Markdown
Collaborator

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

@nodejs-github-bot

nodejs-github-bot commented Jan 29, 2024

Copy link
Copy Markdown
Collaborator

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

@nodejs-github-bot

nodejs-github-bot commented Jan 30, 2024

Copy link
Copy Markdown
Collaborator

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

@MoLowMoLow added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue Add this label to land a pull request using GitHub Actions. labels Jan 31, 2024
@nodejs-github-botnodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jan 31, 2024
@nodejs-github-bot
nodejs-github-bot merged commit a501315 into nodejs:mainJan 31, 2024
@nodejs-github-bot

nodejs-github-bot commented Jan 31, 2024

Copy link
Copy Markdown
Collaborator

Landed in a501315

@MoLow
MoLow deleted the fix-no-colors branch January 31, 2024 08:35
@GulajavaMinistudioGulajavaMinistudio mentioned this pull request Jan 31, 2024
Merged
rdw-msft pushed a commit to rdw-msft/node that referenced this pull request Feb 9, 2024
@MoLow@rdw-msft
repl: fix NO_COLORS env var is ignored
ddd157b
PR-URL: nodejs#51568
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
targos pushed a commit that referenced this pull request Feb 15, 2024
@MoLow@targos
repl: fix NO_COLORS env var is ignored
3e579ab
PR-URL: #51568
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@marco-ippolitomarco-ippolito mentioned this pull request Mar 1, 2024
Merged
richardlau pushed a commit that referenced this pull request Mar 25, 2024
@MoLow@richardlau
repl: fix NO_COLORS env var is ignored
f94336f
PR-URL: #51568
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@richardlaurichardlau mentioned this pull request Mar 25, 2024
Merged
@richardlaurichardlau mentioned this pull request Jan 7, 2025
Closed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@BridgeARBridgeARBridgeAR approved these changes
@addaleaxaddaleaxaddaleax approved these changes
@benjamingrbenjamingrbenjamingr approved these changes
@lpincalpincalpinca approved these changes

Assignees

No one assigned

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.replIssues and PRs related to the REPL subsystem.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@MoLow@nodejs-github-bot@anonrig@addaleax@benjamingr@lpinca@BridgeAR