NO_COLOR suppresses non-color attributes, not just color

[levitte]

NO_COLOR suppresses non-color attributes, not just color

Summary

When NO_COLOR is set, anstream strips all ANSI escape sequences —
bold, dim, italic, underline, reverse, strikethrough, etc. — not only
color. This diverges from no-color.org
(or its source),
whose FAQ is explicit:

Q: Should the presence of NO_COLOR disable other styling such as
bold, underline, and italic?

A: No. This standard only signals the user's intention regarding
adding ANSI color to text output.

Reproduction

NO_COLOR resolves to ColorChoice::Never → StripStream, whose
StripBytes engine (driven by anstyle_parse) discards every CSI escape
without inspecting its SGR parameters. Verified on anstream 1.0.0:

use std::io::Write as _;

let mut out = anstream::AutoStream::new(Vec::new(), anstream::ColorChoice::Never);
write!(out, "\x1b[1mbold\x1b[22m \x1b[4munderline\x1b[24m \x1b[31mred\x1b[39m").unwrap();
assert_eq!(String::from_utf8(out.into_inner()).unwrap(), "bold underline red");

Bold and underline are stripped along with the red color. Per the FAQ, only
color should be suppressed; the expected output is
\x1b[1mbold\x1b[22m \x1b[4munderline\x1b[24m red.

Suggested direction

A spec-correct color-only strip needs to distinguish color SGR parameters
from attribute parameters and re-emit only the latter. Most of the
primitives for this already live in the workspace: anstyle::Style
separates fg/bg/underline-color from effects, and anstyle-parse
already produces the SGR parameter list via csi_dispatch. The missing
piece is a decode step (param list → Style delta) so a strip adapter can
track the active style and render only effects. If a shared decoder
lived in anstyle, other consumers could reuse it for the same spec
alignment — but the shape is of course the maintainers' call.

One design point worth flagging: ColorChoice::Never is reached today for
NO_COLOR, for non-terminals, and for CLICOLOR disabled. The FAQ
constrains only the NO_COLOR case, so it may be cleanest for NO_COLOR
to resolve to a color-only path while the other Never callers keep the
full strip (pipe-to-file output today is fully flat, which is arguably
desirable for logs).

Notes

• The empty-string trigger is already handled correctly upstream:
  anstyle_query::no_color() is non_empty(NO_COLOR), so NO_COLOR=""
  disables nothing, per spec. This issue is only about the scope of
  suppression.
• This cascades to dependents routing through anstream (env_logger,
  clap with its color feature), which would inherit the fix.
• crossterm is a working example of color-only NO_COLOR handling —
  it gates Colored (fg/bg/underline-color) but emits SetAttributes
  unconditionally.
• Related: Supporting FORCE_COLOR as an alias to CLICOLOR_FORCE #192 (env-var support context).

Versioning

I'd frame this as a bug fix (patch bump) with a one-line CHANGELOG note
acknowledging the observable output change, but defer to the project's
own version policy.

[epage]

I'd be curious what others do when they support NO_COLOR. Generally, what people want to control when controlling colors is controlling ansi escape codes and limiting this to literal color would run counter to that.

In general, I would incline to close this as "the spec is wrong".

[levitte]

I disagree with the stance that "the spec is wrong", and my reasoning is very simple: bold, underline, italics and so forth are not colors, so it stands to reason that they should be unaffected by NO_COLOR, given that name.

For me personally, it comes down to color vision (I'm red-green impaired), and the fact that I prefer light-mode terminals (again, visuals, I find dark-mode harder to read, especially in daylight).
Many programs have some coloring artifact that are obviously geared for dark-mode, and that look horrible in light-mode. However, the use of bold, italics and underline is actually helpful, even with my impairment!

[levitte]

(crossterm, as mentioned, does follow the spec)

[epage]

If only 3-bit color is being used, you have complete control over that in your terminal.

[levitte]

Not sure how to enforce that.

Either way, I'm willing to try to make a PR for this, would that be unwelcome?

[epage]

I lean towards not doing this. While there are use cases for this change, with the existing mitigations for them, it doesn't to outweigh expectations / benefits for the current implementation.