Skip to content

fix: honor NO_COLOR specification#1067

Open
vkorenev wants to merge 2 commits into
trunk-rs:mainfrom
vkorenev:fix/no-color-env
Open

fix: honor NO_COLOR specification#1067
vkorenev wants to merge 2 commits into
trunk-rs:mainfrom
vkorenev:fix/no-color-env

Conversation

@vkorenev
Copy link
Copy Markdown

@vkorenev vkorenev commented May 7, 2026

Fixes #1065

@ctron
Copy link
Copy Markdown
Collaborator

ctron commented May 8, 2026

Thanks for the PR. I'm not sure this is the right approach though. This adds another parser (based on the env-var). This parser would, to my understanding, also set "no color" in the case the value is "0", which is clearly against what no-color.org shows in their example.

I think the implementation could look something like:

fn no_color_value(s: &str) -> Result<bool, String> {
    Ok(!matches!(s.to_ascii_lowercase().as_str(), "false" | "0"))
}

#[derive(Parser)]
struct Args {
    #[arg(
        long,
        env = "NO_COLOR",
        action = ArgAction::Set,
        num_args = 0..=1,
        require_equals = true,
        default_missing_value = "true",
        default_value_t = false,
        value_parser = no_color_value,
    )]
    no_color: bool,
}

@vkorenev
Copy link
Copy Markdown
Author

vkorenev commented May 9, 2026

Thanks for reviewing!

That's the https://no-color.org/ example:

	char *no_color = getenv("NO_COLOR");
	bool color = true;

	if (no_color != NULL && no_color[0] != '\0')
		color = false;

This checks for a null-terminated empty string, which is distinct from the string "0" (a string containing the character zero). This is consistent with the specification's description::

Command-line software which adds ANSI color to its output by default should check for a NO_COLOR environment variable that, when present and not an empty string (regardless of its value), prevents the addition of ANSI color.

Everything else makes sense. I'll update the PR.

@vkorenev
Copy link
Copy Markdown
Author

vkorenev commented May 9, 2026

On second thought, I think that having --no-color=false actually disable color output is counterintuitive. Maybe we should use clap::builder::FalseyValueParser as the value_parser instead, even if it means deviating from the https://no-color.org/ spec.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Trunk rejects NO_COLOR=1 with invalid value '1' for '--no-color'

2 participants