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

Allow good/warning/error/hint color to be customized #3234

Closed
2 tasks done
milesj opened this issue Dec 31, 2021 · 65 comments · Fixed by #5094
Closed
2 tasks done

Allow good/warning/error/hint color to be customized #3234

milesj opened this issue Dec 31, 2021 · 65 comments · Fixed by #5094
Labels
A-help Area: documentation, including docs.rs, readme, examples, etc... C-enhancement Category: Raise on the bar on expectations S-blocked Status: Blocked on something else such as an RFC or other implementation work.
Milestone

Comments

@milesj
Copy link

milesj commented Dec 31, 2021

Please complete the following tasks

  • I have searched the discussions
  • I have searched the existing issues

Clap Version

3.0.0-rc.9

Describe your use case

I would like to use colored help/errors/etc in our CLI, but would like to change the colors to match our "brand".

Describe the solution you'd like

Provide a setting for good/warning/error/hint that allows the ANSI color code to be provided, based on the 256 table (or maybe 16m if you're crazy). https://upload.wikimedia.org/wikipedia/commons/1/15/Xterm_256color_chart.svg

It may be better to name the colors based on their actual usage, like arg/bin/label, etc.

Alternatives, if applicable

No response

Additional Context

No response

@milesj milesj added the C-enhancement Category: Raise on the bar on expectations label Dec 31, 2021
@epage epage added A-help Area: documentation, including docs.rs, readme, examples, etc... S-triage Status: New; needs maintainer attention. labels Dec 31, 2021
@epage
Copy link
Member

epage commented Dec 31, 2021

One challenge with this is the API. We can't accept ANSI escape codes because clap3 intentionally added support for non-ANSI Windows terminals. We could copy the enums for color selections but we'd also need styling and the ability to compose them together. We end up starting to duplicate a decent chunk of a terminal styling API which we are hesitant to do (#1790 is a bit more extreme of an example).

We've been instead preferring for feedback on #2963 for improving our defaults and #2914 for allowing people to more thoroughly customize things when those defaults don't work.

It may be better to name the colors based on their actual usage, like arg/bin/label, etc.

We've been moving in that direction. Originally, it was named after the color but the color also implied some other styling with it. The next step is for us to split off --help parts from abusing the existing semantics and giving them their own. Since this is internal, this isn't too much of a priority though will be addressed as part of #2963 and #2389.

would like to change the colors to match our "brand".

Mind elaborating what is "ok" and what runs counter to your "brand"? In what way does it run counter?

@milesj
Copy link
Author

milesj commented Jan 8, 2022

@epage That all makes sense. I've had enough experience with ANSI and CLI tools so I totally get it.

As for the branding piece, the colors are shades of purple/blue/teal. Here's an example of the log output.

Screen Shot 2022-01-08 at 3 07 30 PM

And this is where the colors are configured if you're curious: https://github.com/milesj/moon/blob/master/crates/logger/src/color.rs

@epage
Copy link
Member

epage commented Jan 11, 2022

If you have any feedback or proposals for #2963, could you post it there?

Also, how important are errors or is the main concern the help?

Also, for anyone finding this issue, something that can help for re-evaluating is examples of precedence for this (like dialoguer) and proposals for how this would work either with older Windows support or why we can justify only supporting ANSI escape codes.

Otherwise, at this time I am closing out this issue, since there is no plan to grow the clap API to support this.

@epage epage closed this as completed Jan 11, 2022
@epage epage added S-wont-fix Status: Closed as there is no plan to fix this and removed S-triage Status: New; needs maintainer attention. labels Jan 11, 2022
@epage
Copy link
Member

epage commented Aug 18, 2022

Since this was closed, I've started work on https://github.com/epage/anstyle which aims to allow color definitions to be used in an API independent of the implementation which will allow users to define their stylesheet.

Blocked on https://github.com/epage/anstyle/issues/14

@epage epage reopened this Aug 18, 2022
@epage epage added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-wont-fix Status: Closed as there is no plan to fix this labels Aug 18, 2022
@epage epage added this to the 4.x milestone Aug 30, 2022
@epage
Copy link
Member

epage commented Sep 21, 2022

From reddit

My preferred solution would be that, when customization is figured out, have the existing color behavior as some sort of easy to select "preset".

We could include presets for color schemes to help get people going.

@nyurik
Copy link
Contributor

nyurik commented Oct 7, 2022

I recently switch to v4, and the lack of help colors is a bit disheartening - I grew fond of seeing colors in all the "cool new Rust-based Linux tools", and that color is what instantly made these tools stand apart from the old GNU tools. Now Clap only uses bold/underline, while continuing to call it "color". I tried to re-enable the legacy colors somehow, but despite searching all the release notes that only mention it in passing, and skimming through a very lengthy discussion on removing colors, I am beginning to suspect it is not (yet) possible.

To avoid breaking existing functionality, I feel v4 should have included the support for existing color styling from the beginning, even if it would require something like #[clap(legacy_color_scheme = true)].

Is this what is planned for the next release? Regardless of the minor grief above, thank you for all the hard work on Clap - it's one of the best crates we have in Rust! :)

@epage
Copy link
Member

epage commented Oct 7, 2022

@nyurik

I tried to re-enable the legacy colors somehow, but despite searching all the release notes that only mention it in passing, and skimming through a very lengthy discussion on removing colors, I am beginning to suspect it is not (yet) possible.

#4132 summarizes all of the help changes and includes workarounds, where possible.

For this case, it says

Users can't rollback until theming support is in (#3234) though they can just disable the coloring with Command::disable_colored_help(true)

@nyurik

legacy_color_scheme

Such a flag is not going to be supported. Instead, our focus will be to theming

@nyurik

Is this what is planned for the next release

We do not commit to specific features for a "next release" as we we release on almost every PR made.

Addressing color support, in general, is a priority though. The main thing blocking theming support is releasing a 1.0 of https://docs.rs/anstyle/latest/anstyle/ which is blocked on getting more feedback on the API / more runtime to make sure the API is good for a 1.0 release, see https://github.com/epage/anstyle/issues/14, see also my earlier comment

As it becomes a chicken and egg problem, my plan has been to implement styling support in #3108 / #1433 as that will at least give me some experience with anstyle to decide when its ready to go 1.0 and be safe to use in other crate's APIs.

@nyurik
Copy link
Contributor

nyurik commented Oct 7, 2022

Thanks @epage for such a great in-depth reply! I think we should modify the CHANGELOG file (and possibly the release v4 comment) -- to make it clearer. I searched for the word "color", and nothing came up. I found this

We've moved to a more neutral palette for highlighting elements (not highlighted above)

which is not a good explanation when a highly visible breaking change is introduced. Perhaps something like this text? I will be happy to make a pull request.

We have removed colored help in favor of bold and underlined text. There is currently no way to re-enable the old color scheme until the styling (#3108 / #1433) is implemented.

@epage
Copy link
Member

epage commented Oct 7, 2022

As I'm trying to focus to get other things done so I can get back to that work, I do not have time to litigate a discussion about how to handle this (sorry, you are partly getting the short end of the stick from some other individuals taking up too much time / attention). As that is the case, we'll leave it as-is for now.

@donovanglover
Copy link

It took me some time to figure this out, but here's an example of adding color to --help.

use clap::builder::styling::{AnsiColor, Effects, Styles};
use clap::Parser;

fn styles() -> Styles {
    Styles::styled()
        .header(AnsiColor::Red.on_default() | Effects::BOLD)
        .usage(AnsiColor::Red.on_default() | Effects::BOLD)
        .literal(AnsiColor::Blue.on_default() | Effects::BOLD)
        .placeholder(AnsiColor::Green.on_default())
}

#[derive(Parser)]
#[command(author, version, about, styles = styles())]
pub struct Cli {
    // ...
}

@murlakatamenka
Copy link

It took me some time to figure this out, but here's an example of adding color to --help.

use clap::builder::styling::{AnsiColor, Effects, Styles};
use clap::Parser;

fn styles() -> Styles {
    Styles::styled()
        .header(AnsiColor::Red.on_default() | Effects::BOLD)
        .usage(AnsiColor::Red.on_default() | Effects::BOLD)
        .literal(AnsiColor::Blue.on_default() | Effects::BOLD)
        .placeholder(AnsiColor::Green.on_default())
}

#[derive(Parser)]
#[command(author, version, about, styles = styles())]
pub struct Cli {
    // ...
}

Piggybacking on this, since Styles::styled is const:

use clap::{
    builder::styling::{AnsiColor as Ansi, Styles},
    Parser,
};

const MY_AWESOME_STYLE: Styles = Styles::styled()
    .header(Ansi::Red.on_default().bold())
    .usage(Ansi::Red.on_default().bold())
    .literal(Ansi::Blue.on_default().bold())
    .placeholder(Ansi::Green.on_default());

#[derive(Parser)]
#[command(styles = MY_AWESOME_STYLE)]
pub struct Cli {
    // ...
}

fn main() {
    let _cli = Cli::parse();
}

Clap v3 Style:

rustic's style:

@MilesCranmer
Copy link

MilesCranmer commented Apr 15, 2024

Here's an example of manually customizing the colors in the help template:

https://github.com/MilesCranmer/rip2/blob/29b46efbff046515360cf5a16bdbae678db90b37/src/args.rs#L8-L83

Both in the text (with anstyle render) and by letting clap do it.

On white background:

Screenshot 2024-04-15 at 12 12 41

And dark:

Screenshot 2024-04-15 at 12 13 15

(Same color scheme as cargo)

@heaths
Copy link

heaths commented Aug 7, 2024

We can't accept ANSI escape codes because clap3 intentionally added support for non-ANSI Windows terminals.

I've started digging into this problem and came across this. As someone who's worked a lot with the Windows Terminal team who added conpty support to both wt.exe and conhost.exe circa Windows 10, as well as helped the GitHub CLI team with a bunch of ANSI color issues, this is solvable. There are a lot of linked issues so I'm not sure if this has been discussed, but effectively, if you call GetConsoleMode() to get the current console mode, OR that with ENABLE_VIRTUAL_TERMINAL_PROCESSING, and call SetConsoleMode(). If that returns non-zero, conpty was enabled and you can safely use ANSI colors.

See https://github.com/microsoft/vswhere/blob/9fd6f2c6df9ccff186412121f2016b81ff4d9e4c/src/vswhere.lib/Console.cpp#L116 or https://github.com/cli/go-gh/blob/25db6b99518c88e03f71dbe9e58397c4cfb62caf/pkg/term/console_windows.go#L12. Any terminal from Windows 10 TH2 (IIRC) in the OS will support this, though third-party solutions may not; however, they should fail that call as well, so you know not to write ANSI escape sequences (or filter them out on write - whichever is easier since parsing them out is fairly trivial e.g., https://github.com/heaths/go-console/blob/d0d8e65f7196273cfb7f2354f48c13beae9d60d8/internal/writer/writer.go#L27).

Or, does this really come down to just being able to theme the colors reliably and easily? There are quite a few linked issues/discussions about this. I've read through a couple. But there are quite a few just about the lack of color at all.

@epage
Copy link
Member

epage commented Aug 7, 2024

@heaths that is a pretty stale comment you are replying to and so a lot of your reply doesn't seem relevant. This issue has long since been resolved, We knew of and take care of enabling ANSI escape codes on Windows.

@heaths
Copy link

heaths commented Aug 8, 2024

v4 still shows non-colored text. Searching the issues, I found this issue and nothing obvious more recent. Is there something more recent? Again reviewing the docs and writing a quick sample, I only see underlined text.

@gibfahn
Copy link
Contributor

gibfahn commented Aug 8, 2024

I believe this is how you style the text: https://docs.rs/clap/latest/clap/struct.Command.html#method.styles

@epage
Copy link
Member

epage commented Aug 8, 2024

@heaths the fact that you saw underlined (and I assume bold) means you were getting ANSI escape codes.

In 4.0.0

We've moved to a more neutral palette for highlighting elements (not highlighted above)

See also #2963 which was linked to from the first post of mine that you quoted from. In CLIs, the term "color" tends to be a placeholder for "any ansi escape codes".

This issue is about customizing it so you don't have to use that color palette and cargo --help is an example of that in practice.

With this issue closed, we can be a bit more "daring" with our choice and I am open to people discussing what would be a good "universal" / "safe" default color palette in its own issue. Note though that changing the color palette would be considered a breaking change according to our guidelines.

@heaths
Copy link

heaths commented Aug 8, 2024

Are their docs on how to change the ANSI escape sequences? I looked prior to replying originally - understanding at least part of this issue was about that - but found nothing. To be honest, I care less about the defaults if it's easy to customize.

@epage
Copy link
Member

epage commented Aug 8, 2024

@heaths they were linked to earlier in #3234 (comment), see https://docs.rs/clap/latest/clap/struct.Command.html#method.styles

@heaths
Copy link

heaths commented Aug 8, 2024

Apologies. I missed that. Any objection to linking that or even adding an example to the tutorial docs? I'd be happy to submit a PR if you're cool with that.

@heaths
Copy link

heaths commented Aug 8, 2024

Actually, looking more carefully through https://docs.rs/clap/latest/clap/_derive/index.html#command-attributes it seems that's not currently possible. I will open a separate issue to perhaps accept a function to supply styles or something, or has that been discussed already as well? I prefer derive to builder myself and would love to more easily take advantage of this behavior.

@epage
Copy link
Member

epage commented Aug 8, 2024

@heaths I've found that each person has their own problem they've run into and "put it in the tutorial" is a common solution proposed. To do that in all cases would make it as hard to find things in the tutorial as the API. I don't have a good answer for where the "line" should be.

However, cargo has adopted custom colors and so it would be reasonable to add color support to our cargo cookbook entry and highlight in the summary that its using custom colors.

Actually, looking more carefully through https://docs.rs/clap/latest/clap/_derive/index.html#command-attributes it seems that's not currently possible. I will open a separate issue to perhaps accept a function to supply styles or something, or has that been discussed already as well?

What in that is giving the impression its not supported? I'm using it in cargo-release
https://github.com/crate-ci/cargo-release/blob/e30b1c6c645700f4189b76130de0896e19f8d0b1/src/bin/cargo-release.rs#L54

If its because the attribute isn't explicitly named, thats because it falls under the catch-all "raw attribute" section, see also #4090

@heaths
Copy link

heaths commented Aug 8, 2024

Fair point about where to draw the line, but seems styles is a pretty major feature to at least warrant an example. As much attention as the lack of colors in v4 got across a few projects, seems like a good idea to have a section or even chapter about.

That thread about raw attributes also points out how there's no mention in the tutorial docs. Clap has an expansive API and it seems that features like this getting called out in the tutorial with links to the to docs could help. Personally, your tutorials are good enough that I typically don't need to dig into the ref docs too much. The only time I remember was when writing my own value parser. I don't imagine I'm alone in this.

epage added a commit to epage/clap that referenced this issue Aug 8, 2024
Inspired by part of the conversation at clap-rs#3234
epage added a commit to epage/clap that referenced this issue Aug 8, 2024
Inspired by part of the conversation at clap-rs#3234
@epage
Copy link
Member

epage commented Aug 8, 2024

#5638 is up to add it to the cookbook. Part of the role of the cookbook is for people to say "I want this feature from this command, how do I do it".

That thread about raw attributes also points out how there's no mention in the tutorial docs.

It is called out in the tutorial, likely added based on that feedback, e.g.

Any Command builder function can be used as an attribute, like Command::next_line_help.

See https://docs.rs/clap/latest/clap/_derive/_tutorial/chapter_1/index.html

I'm likely to write a documentation generator based on cargo doc --json but haven't gotten to it yet. If someone wants to take that on, I can give them rough guidelines of what I expect (and what is left undecided).

@heaths
Copy link

heaths commented Aug 8, 2024

I'm likely to write a documentation generator based on cargo doc --json but haven't gotten to it yet. If someone wants to take that on, I can give them rough guidelines of what I expect (and what is left undecided).

Are these written down somewhere, or do we want to take that to a new issue or zulip? I might be game.

@epage
Copy link
Member

epage commented Aug 8, 2024

The current write up is at #4090 (comment) but moving to an issue could make it easier to track

@heaths
Copy link

heaths commented Aug 9, 2024

#5639

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-help Area: documentation, including docs.rs, readme, examples, etc... C-enhancement Category: Raise on the bar on expectations S-blocked Status: Blocked on something else such as an RFC or other implementation work.
Projects
None yet
Development

Successfully merging a pull request may close this issue.