-
Notifications
You must be signed in to change notification settings - Fork 985
feat(cli): Add color to clap help/errors #4551
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We might indeed want to address https://rust-lang.zulipchat.com/#narrow/channel/490103-t-rustup/topic/More.20rustup.2Fcargo.2Frustc.20consistency/near/545410015 in addition.
Otherwise this looks pretty nice, thanks!
|
Everything should now be styled. |
This is based on review feedback from rust-lang#4551. I experimented with a `clippy.toml` file for "blessing" some of these, like `errors`, but for some reason I wasn't able to get it to work.
src/cli/help.rs
Outdated
| use anstyle::Style; | ||
| use clap_cargo::style::{HEADER, LITERAL, PLACEHOLDER}; | ||
|
|
||
| const SUBHEADER: Style = Style::new().bold(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: sorry, one more that I forgot in the last round: please put this at the bottom.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Went ahead and did this but this is one of the rare cases where it feels better at the top to me.
In general, the way I view organizing a source file is it should act like a tree with the top item(s) at the top and branch out from there, so the top item(s) act as a ToC and provide context. In that case, constants are noise. However, in this case, this has all of the styling elements together and now we're splitting them up.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't find that a very strong argument. Lots of modules where some items are imported while other items are defined near the bottom.
This is based on review feedback from rust-lang#4551. I experimented with a `clippy.toml` file for "blessing" some of these, like `errors`, but for some reason I wasn't able to get it to work.
This is based on review feedback from rust-lang#4551. I experimented with a `clippy.toml` file for "blessing" some of these, like `errors`, but for some reason I wasn't able to get it to work.
This is based on review feedback from #4551. I experimented with a `clippy.toml` file for "blessing" some of these, like `errors`, but for some reason I wasn't able to get it to work.
This adds styling to clap help and errors to make it easier to scan. This uses Cargo's styling (via
clap-cargo) for consistency across the project.rustupandcargoand not plumbing commands likerustfmtandrustc.To ensure consistency, the Discussion sections were updated.
rustup completionscolor-printto do the styling at compile time but that would require duplicating Cargo's styling which would have been harder to maintain. Instead, we style at runtime so we can useclap-cargo.