-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Allow customizing styling of [default]
, [possible values]
, [env]
in arg description
#5093
Comments
Hi, we can also add I am in favor for:
The reason is the stay consistent with how For that, we would need to add one more field to the struct I looked into the code and found that the modification should occures in I am ready to work on this and to submit a PR but I have to admit that I am a bit confused about the mixed used of @epage Can you guide me on this precise point please ? And also, is there another place where I need to edit code ? |
You don't need to worry about it. The styling design that clap uses follows the idea of decoupling the presentation from the medium its being rendered to. We style everything with colors as needed. When we output, instead of
While I understand the principle, I think we also need to keep in mind how all of this will look. When I was styling Cargo's output, I had considered using |
Hi @epage, I started to work on it. I add a A) B) C) D) E) In the samples above, I used this style: fn get_cli_styles() -> Styles {
use clap::builder::styling::AnsiColor;
Styles::styled()
.header(AnsiColor::Green.on_default().bold())
.usage(AnsiColor::Green.on_default().bold())
.literal(AnsiColor::Cyan.on_default().bold())
.placeholder(AnsiColor::Cyan.on_default())
.error(AnsiColor::Red.on_default().bold())
.valid(AnsiColor::Green.on_default().bold())
.invalid(AnsiColor::Yellow.on_default().bold())
.spec(AnsiColor::Green.on_default())
} Personall, I think I prefer C) and E). Also, while working on it, I can easily make the styling even more customizable by allowing the user to specify a style for each "spec", I mean for fn get_cli_styles() -> Styles {
use clap::builder::styling::AnsiColor;
Styles::styled()
.header(AnsiColor::Green.on_default().bold())
.usage(AnsiColor::Green.on_default().bold())
.literal(AnsiColor::Cyan.on_default().bold())
.placeholder(AnsiColor::Cyan.on_default())
.error(AnsiColor::Red.on_default().bold())
.valid(AnsiColor::Green.on_default().bold())
.invalid(AnsiColor::Yellow.on_default().bold())
.spec(AnsiColor::Green.on_default()) // set fallback style for `[env], [default], [possible values], [aliases] and [short aliases]`
.env(AnsiColor::Magenta.on_default() // set style just for `env` keyword.
} I can do this but it feels a bit too much maybe ?
|
That still leaves us trying to balance
If we do (D) with Maybe one way to balance this is to have
Let's keep it simple for the first release and see where to go. This works towards our balancing of functionality, best practices, binary size, and compile times. The best practices is referring to the fact that we shouldn't be encouraging people to make their output a rainbow. imo we need strong use cases to show why more specific styling is needed.
I think prefixing with "inline" helps distinguish this from our possible-values output for As for For brainstorming
Looking at our compatibility expectations, we probably need to start with a default style. I'm mixed about whether changing that style would constitute a minor or a major release. At the moment, I'm leaning towards major. If/when we get to that point, italic would be a bad default for us to use because it is aliased to Invert in a lot of terminals which stands out glaringly. I somewhat favor "dimmed" but I know some people who say that is unreadable on their terminal. Likely, we'd stick with default unless we decide to add colors into our default styling. |
Hi @epage, I implemented according to your feedback, it passed the I went with I just have one problem with the styling of "Possible Values:" (for enumerated values / multiple choices): For now I left it as it is probabling at least a minor release. I am waiting for your feedback, in the meantimes I will do my PR. |
As a heads up, I'm behind on a lot of my PRs and Issues. It may take a while before I can get back to this. |
as a note, Deno would be interested in having this; is there anything we can help out with to push this forward? |
Still very far behind. |
Please complete the following tasks
Clap Version
4.3.3
Describe your use case
From #3234 (comment)
Describe the solution you'd like
Allow styling to either help it stand out from the rest of the description
Alternatives, if applicable
No response
Additional Context
From #3234 (comment)
So long as we only use new style definitions, it is not a major or minor breaking change according to our support policy. See https://github.com/clap-rs/clap/blob/master/CONTRIBUTING.md#compatibility-expectations
The text was updated successfully, but these errors were encountered: