Open mchernyavsky opened 3 years ago
The main problem here is the circular nature of this.
--color=always
, we need to parse the CLI but the error is coming from our CLI parserCARGO_TERM_COLOR
, we need the Config
configured which requires the CLI to be parsed.We can't workaround it for --color=always
but we can for CARGO_TERM_COLOR
by directly looking up the value and passing that to clap. We'd probably want to extract the value parser out of set_color_choice
into a std::str::FromStr
impl on ColorChoice
.
This would make it so fn cli()
would have something like
let color_choice = std::env::var_os("CARGO_TERM_COLOR");
let color_choice = color_choice.and_then(|c| c.parse::<ColorChoice>().ok());
let color_choice = match color_choice {
Some(ColorChoice::CargoAuto) | None => clap::ColorChoice::Auto,
Some(ColorChoice::Always) => clap::ColorChoice::Always,
Some(ColorChoice::Never) => clap::ColorChoice::Never,
};
This intentionally ignores the errors since it isn't worth reporting at this stage.
The main question is if this is worth it or not.
--color
alone.To handle --color
, we'd need to be able to parse the command line to get those which requires knowing what level of styling to use. We could say "parse the command line, take the result, and strip the message based on our own decision". However, --help
and errors short-circuits CLI parsing. We'd have to parse, see if an error gets back, and then re-parse using the "ignore errors" setting which is a very brittle feature in clap.
I'd propose that we reject --color
and close this as "resolved" with the merge of #13463
I'd propose that we reject
--color
and close this as "resolved" with the merge of #13463
That sounds fair. Should we document this unfortunate truth somewhere?
I untagged it; unsure why github closed it.
Should we document this unfortunate truth somewhere?
I'm for that but unsure where it would work that it both helps the user and doesn't overwhelm them with details when they don't care (CLI parsing is a bit niche).
I untagged it; unsure why github closed it.
https://github.com/rust-lang/cargo/pull/13463/commits/67474259e9893fdfa3a9f98148dc871e3c970e31
The close keyword also works in commit messages 😈.
FWIW cargo bench
still prints with color with despite CARGO_TERM_COLOR=never
and --color=never
.
For some project with benches
$ CARGO_TERM_COLOR=never cargo bench --benches --quiet --color=never
I used my project.
Using cargo 1.79.0 on Ubuntu 22, target x86_64-unknown-linux-gnu
.
@jtmoon79 thanks for the report. That is a different issue than this. Test harness (by default it's libtest from rustc) usually provide their own way to customize console output. Those arguments could be passed after double dashes --
, e.g. cargo test -- --nocapture
. Cargo makes no assumption to test/benchmark harness so it doesn't control the color of the output. See https://github.com/rust-lang/cargo/issues/1983 for the relevant issue.
Problem
^title
I expect that with
--color=always
option (orCARGO_TERM_COLOR=always
environment variable) the output will contain expected ANSI escape codes.Steps
cargo build --color=always --unknown &> out.txt
out.txt
will contain the following output (without ANSI escape codes):Notes
Output of
cargo version
: