Skip to content
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

doc: fix desc for NO_COLOR and NODE_DISABLE_COLORS env vars in cli.md #56486

Closed
wants to merge 1 commit into from

Conversation

veganaize
Copy link

@veganaize veganaize commented Jan 5, 2025

Improve wording for NO_COLOR and NODE_DISABLE_COLORS environment variables in cli.md api doc.

Closes #56485

Improve wording for `NO_COLOR` and `NODE_DISABLE_COLORS` environment variables.
@nodejs-github-bot nodejs-github-bot added cli Issues and PRs related to the Node.js command line interface. doc Issues and PRs related to the documentations. labels Jan 5, 2025
@veganaize veganaize changed the title doc: improve wording for NO_COLOR and NODE_DISABLE_COLORS environment variables in cli.md doc: improve wording for NO_COLOR and NODE_DISABLE_COLORS env vars in cli.md Jan 5, 2025
@veganaize veganaize changed the title doc: improve wording for NO_COLOR and NODE_DISABLE_COLORS env vars in cli.md doc: fix desc for NO_COLOR and NODE_DISABLE_COLORS env vars in cli.md Jan 5, 2025
@@ -3352,7 +3352,8 @@ propagation.

### `NO_COLOR=<any>`

[`NO_COLOR`][] is an alias for `NODE_DISABLE_COLORS`. The value of the
[`NO_COLOR`][] When set, disables color only for the command-line. The REPL will
continue to output color unless `NODE_DISABLE_COLORS` is set. The value of the
Copy link
Member

Choose a reason for hiding this comment

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

I don't believe this is correct. NO_COLOR should act identical to NODE_DISABLE_COLORS

Copy link
Author

Choose a reason for hiding this comment

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

It doesn't in the cmd.exe shell of Windows 8.1 (x64), nor in the terminal on Debian 12, from my testing.

Copy link
Member

Choose a reason for hiding this comment

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

I'm not certain that windows 8.1 is still a supported version. Not sure about Debian 12. In either case, however, the expectation is that these are aliases with identical behavior, so any variance from that would be a bug in the implementation rather than a bug in the docs.

Copy link
Author

Choose a reason for hiding this comment

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

Please test it for yourself, and see. This is the behavior back through v14, to my knowledge. Debian 12 is the current stable version of Debian.

Copy link
Member

Choose a reason for hiding this comment

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

I'm not questioning whether it works or not, I'm saying that it's supposed to. So if it doesn't the bug is not in the docs but the implementation.

Copy link
Author

Choose a reason for hiding this comment

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

The related issue is #56485

Copy link
Author

Choose a reason for hiding this comment

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

I can confirm that the output is working correctly (ie. NO_COLOR acting as alias for NODE_DISABLE_COLORS) in Node.js v20.9 on Arch Linux.

So that implies the last LTS version the issue applies to is v18 (which I tested against the latest minor version of, and NO_COLOR still outputs color in the REPL.)

v18 is still under current LTS support (including "experimental" support for Windows 8.1).

Copy link
Author

Choose a reason for hiding this comment

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

Created new issue #56509 and closed this PR.

@veganaize veganaize closed this Jan 7, 2025
@veganaize veganaize deleted the patch-1 branch January 7, 2025 21:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli Issues and PRs related to the Node.js command line interface. doc Issues and PRs related to the documentations.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

NO_COLOR and NODE_DISABLE_COLORS described incorrectly in CLI api doc
3 participants