-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Cargo and rustc are using different tones of red for errors on the terminal #12740
Comments
Thanks for the report! We're aware of this and have #12627 for tracking one of the approach. See also #2290 (comment) for a short summary. I lean to waiting for #12627 but also fine with manual tweaks. |
Without investigating it by myself, do you know since which version this happened, or it has been there for a while? |
I have no idea. I only just realized this when working on the messages for ui_test. It might have been like this for a while, it might have been hidden by me using a different terminal/configuration, it might be recent. |
I went back through time a bit -- this is definitely a very long-standing issue. I see the difference even with Rust 1.40. |
Glad to see this wasn't something I broke recently as I've been making changes to how we do styling :) #12578 added styling to clap and #12655 made us use shared definitions to ensure we'd stay in sync which also made our style choices easier to audit. Cargo uses bold+red and a lot of has likely haven't noticed the discrepancy because bold+red gets colored the same as bright_red in a lot of themes. |
We should probably do an audit of all colors used with their intended use and see where it'd work best to align. Note that as follow ups to #12578 and #12655, cargo-fmt and cargo-clippy were also updated to align with cargo. I'm tempted to do this also to rustup but they hadn't upgrade to clap v4 last I checked. |
@epage Oh we have just done that! (Thanks @djc for your rust-lang/rustup#3444!) |
feat: Add `rustc` style errors for manifest parsing #12235 is tracking user control over warnings. One part of that is making `cargo`'s diagnostic system output messages in the style of `rustc`. To make it so that `cargo` doesn't have to manage a formatter and renderer, I decided to use [`annotate-snippets`](https://crates.io/crates/annotate-snippets), which matches `rustc`'s style well (at one time it was meant to be the formatted for `rustc`). To get this work kicked off, it was suggested to me that we should start with styling manifest parsing errors, and that is what his PR does. What manifest parsing errors look like after this change: ![image](https://github.com/rust-lang/cargo/assets/23045215/0eb388b9-8d72-48ad-84a9-585160995078) --- Note: `cargo` does not currently match `rustc` in color (#12740), `rustc` specifies their colors [here](https://github.com/rust-lang/rust/blob/740cea81d6e34fc03c4495890e29f1c5ecb40b96/compiler/rustc_errors/src/lib.rs#L1755-L1768) and [here](https://github.com/rust-lang/rust/blob/740cea81d6e34fc03c4495890e29f1c5ecb40b96/compiler/rustc_errors/src/emitter.rs#L2689-L2723). I used `annotate-snippets` default colors which match what `rustc` currently uses for colors. I did this as a stopgap while I work to unify the colors used between `rustc` and `cargo`. --- Note: the error messages come directly from `toml` and can be quite long. It would be nice if we could put some of the message below the highlight but this would require changes to `toml`. Example: ``` error: invalid type: integer `3` --> Cargo.toml:7:7 | 7 | bar = 3 | ^ expected a version string like "0.9.8" or a detailed dependency like { version = "0.9.8" } | ```
feat: Add `rustc` style errors for manifest parsing #12235 is tracking user control over warnings. One part of that is making `cargo`'s diagnostic system output messages in the style of `rustc`. To make it so that `cargo` doesn't have to manage a formatter and renderer, I decided to use [`annotate-snippets`](https://crates.io/crates/annotate-snippets), which matches `rustc`'s style well (at one time it was meant to be the formatted for `rustc`). To get this work kicked off, it was suggested to me that we should start with styling manifest parsing errors, and that is what his PR does. What manifest parsing errors look like after this change: ![image](https://github.com/rust-lang/cargo/assets/23045215/0eb388b9-8d72-48ad-84a9-585160995078) --- Note: `cargo` does not currently match `rustc` in color (#12740), `rustc` specifies their colors [here](https://github.com/rust-lang/rust/blob/740cea81d6e34fc03c4495890e29f1c5ecb40b96/compiler/rustc_errors/src/lib.rs#L1755-L1768) and [here](https://github.com/rust-lang/rust/blob/740cea81d6e34fc03c4495890e29f1c5ecb40b96/compiler/rustc_errors/src/emitter.rs#L2689-L2723). I used `annotate-snippets` default colors which match what `rustc` currently uses for colors. I did this as a stopgap while I work to unify the colors used between `rustc` and `cargo`. --- Note: the error messages come directly from `toml` and can be quite long. It would be nice if we could put some of the message below the highlight but this would require changes to `toml`. Example: ``` error: invalid type: integer `3` --> Cargo.toml:7:7 | 7 | bar = 3 | ^ expected a version string like "0.9.8" or a detailed dependency like { version = "0.9.8" } | ```
Problem
Cargo and rustc are not consistent in terms of how they are coloring errors on the terminal.
Steps
cargo check
on a project with build failures.As you can see, the two "error" are using two different shades of red. In terms of the
colorize
crate, rustc is usingbright_red
and cargo is usingred
.Possible Solution(s)
One of cargo or rustc should switch to the other tone of red. I have a slight preference for "bright red", that's why I reported this as a cargo issue.
Notes
No response
Version
The text was updated successfully, but these errors were encountered: