clap-rs / clap

A full featured, fast Command Line Argument Parser for Rust
docs.rs/clap
Apache License 2.0
14.38k stars 1.05k forks source link

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

Closed milesj closed 1 year ago

milesj commented 2 years ago

Please complete the following tasks

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

heaths commented 3 months ago

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 commented 3 months ago

@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 commented 3 months ago

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 commented 3 months ago

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

epage commented 3 months ago

@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 commented 3 months ago

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 commented 3 months ago

@heaths they were linked to earlier in https://github.com/clap-rs/clap/issues/3234#issuecomment-2275752109, see https://docs.rs/clap/latest/clap/struct.Command.html#method.styles

heaths commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

@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 commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

The current write up is at https://github.com/clap-rs/clap/discussions/4090#discussioncomment-6973754 but moving to an issue could make it easier to track

heaths commented 3 months ago

https://github.com/clap-rs/clap/issues/5639