Skip to content
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

Open
RalfJung opened this issue Sep 26, 2023 · 7 comments
Open
Labels
A-console-output Area: Terminal output, colors, progress bar, etc. C-bug Category: bug S-needs-info Status: Needs more info, such as a reproduction or more background for a feature request.

Comments

@RalfJung
Copy link
Member

RalfJung commented Sep 26, 2023

Problem

Cargo and rustc are not consistent in terms of how they are coloring errors on the terminal.

Steps

  1. Run cargo check on a project with build failures.

image

As you can see, the two "error" are using two different shades of red. In terms of the colorize crate, rustc is using bright_red and cargo is using red.

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

$ cargo version --verbose
cargo 1.72.0 (103a7ff2e 2023-08-15)
release: 1.72.0
commit-hash: 103a7ff2ee7678d34f34d778614c5eb2525ae9de
commit-date: 2023-08-15
host: x86_64-unknown-linux-gnu
libgit2: 1.6.4 (sys:0.17.2 vendored)
libcurl: 8.1.2-DEV (sys:0.4.63+curl-8.1.2 vendored ssl:OpenSSL/1.1.1u)
ssl: OpenSSL 1.1.1u  30 May 2023
os: Debian n/a (trixie) [64-bit]
@RalfJung RalfJung added C-bug Category: bug S-triage Status: This issue is waiting on initial triage. labels Sep 26, 2023
@weihanglo
Copy link
Member

weihanglo commented Sep 26, 2023

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.

@weihanglo weihanglo added the A-console-output Area: Terminal output, colors, progress bar, etc. label Sep 26, 2023
@weihanglo
Copy link
Member

Without investigating it by myself, do you know since which version this happened, or it has been there for a while?

@weihanglo weihanglo added S-needs-team-input Status: Needs input from team on whether/how to proceed. and removed S-triage Status: This issue is waiting on initial triage. labels Sep 26, 2023
@RalfJung
Copy link
Member Author

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.

@RalfJung
Copy link
Member Author

I went back through time a bit -- this is definitely a very long-standing issue. I see the difference even with Rust 1.40.

@epage
Copy link
Contributor

epage commented Sep 26, 2023

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.

@epage
Copy link
Contributor

epage commented Sep 26, 2023

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 epage added S-needs-info Status: Needs more info, such as a reproduction or more background for a feature request. and removed S-needs-team-input Status: Needs input from team on whether/how to proceed. labels Sep 26, 2023
@rami3l
Copy link
Member

rami3l commented Sep 26, 2023

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!)

bors added a commit that referenced this issue Jan 10, 2024
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" }
  |
```
bors added a commit that referenced this issue Jan 11, 2024
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" }
  |
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-console-output Area: Terminal output, colors, progress bar, etc. C-bug Category: bug S-needs-info Status: Needs more info, such as a reproduction or more background for a feature request.
Projects
None yet
Development

No branches or pull requests

4 participants