Bug: Clap doesn't always respect `--color always`

[Cretezy]

Description

jj branch create "" --color always returns a clap error without color if executed in a non-terminal.

Steps to Reproduce the Problem

# has color
jj branch create "" --color always
# no color
jj branch create "" --color always > /dev/null`

or with code:

use std::process::Command;

fn main() {
    // Has color
    let output = Command::new("jj")
        .args(["branch", "delete", "non-existant", "--color", "always"])
        .output()
        .unwrap();

    println!("{}", String::from_utf8_lossy(&output.stderr));

    // No color
    let output = Command::new("jj")
        .args(["branch", "create", "", "--color", "always"])
        .output()
        .unwrap();

    println!("{}", String::from_utf8_lossy(&output.stderr));
}

Also tried with .env("CLICOLOR_FORCE", "1"), which is the envvar to force clap to have color.

Expected Behavior

Both outputs are colored.

Actual Behavior

Only the second output has color (first is a clap error)

Specifications

• Platform: Archlinux
• Version: 0.17.1/main

[ilyagr]

I wonder whether this is easily fixable without making it fail in the other direction (printing color when you request no color). If it's a choice between the two, the current behavior (no color if jj is confused) seems better to me.

[Cretezy]

I don't think it should fallback to color, only clap should use the --color always or the color config option.

[martinvonz]

I think what Ilya is saying is that the CLI error on jj branch "" makes it so that clap never gets to the --color always part.

[Cretezy]

For reference, this seems to be where color is added: https://github.com/martinvonz/jj/blob/7e6a968415214bec164d23635a565ce1c4ec07aa/cli/src/command_error.rs#L752