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

fix(help)!: Consoldiate color settings #2845

Merged
merged 1 commit into from
Oct 11, 2021
Merged

Conversation

epage
Copy link
Member

@epage epage commented Oct 11, 2021

A lot of users expected color feature flag and ColorAuto etc to
control all colors. Having this extra flag around is easy to miss and
adds to our overall settings bloat, making it harder to find settings
people want.

This completely removes it, rather than make it deprecated like
functions in #2617, because there is extra work to mark things
deprecated as Settings and we should decide on our strategy first before
investing time in addressing that issue.

Fixes #2806

A lot of users expected `color` feature flag and `ColorAuto` etc to
control all colors.  Having this extra flag around is easy to miss and
adds to our overall settings bloat, making it harder to find settings
people want.

This completely removes it, rather than make it deprecated like
functions in clap-rs#2617, because there is extra work to mark things
deprecated as Settings and we should decide on our strategy first before
investing time in addressing that issue.

Fixes clap-rs#2806
Copy link
Member

@pksunkara pksunkara left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bors r+

@bors
Copy link
Contributor

bors bot commented Oct 11, 2021

Build succeeded:

  • CI

@bors bors bot merged commit 562e64c into clap-rs:master Oct 11, 2021
@epage epage deleted the coloredhelp branch October 11, 2021 15:26
@dbrgn
Copy link

dbrgn commented Oct 17, 2021

Hi, I was surprised by this change. I'm exactly the person mentioned by @pksunkara in #2055 that wants colors in error messages, but not in help pages. That's why I enabled the colors feature, but did not opt in to the ColoredHelp feature.

I'm the maintainer for tealdeer and am currently porting the codebase to clap v3. When the colors were enabled by default during the update from beta 4 to beta 5 we discussed whether we should keep the help colors or not. Reasons why we decided against it:

  • I'm not really sure if colored help texts are an improvement in readability, or just a distraction. I myself am not a big fan of them for that reason.
  • Whether it looks good or not depends on the terminal settings. It might look good on some terminals, while it might look bad on others. (I know that this is a "the user can fix the colors" kind of thing, but some terminals only support very few colors, and this whole topic is something that users previously did not need to care about.)
  • As a user, I can disable help colors by setting NO_COLOR=1, but this will disable all colors, which may not be intended.
  • The tealdeer CLI program outputs colored content. This output can be styled through the config file. If the help text now contains colors as well, users will want to configure those colors. This results in feature requests for tealdeer (I wouldn't want to implement such a feature) and it would also result in a feature request for clap (right now colors aren't configurable, right?).

In contrast to the help texts, I think that colored error messages do improve the user experience. However, if there's no way to turn off colored help texts, I'll turn off colored error messages as well.

Please reconsider.

@pksunkara
Copy link
Member

pksunkara commented Oct 17, 2021

DisableColoredHelp setting can be added if @epage agrees.

dbrgn added a commit to tealdeer-rs/tealdeer that referenced this pull request Oct 17, 2021
May be re-introduced if clap adds a `DisableColoredHelp` setting.

Relevant discussion:

- clap-rs/clap#2845 (comment)
- #108 (comment)
@epage
Copy link
Member Author

epage commented Oct 18, 2021

I've split this conversation out in #2906 so we don't lose it in the comments of a PR and have a little more flexibility in how to handle it.

epage pushed a commit that referenced this pull request Dec 7, 2021
Until we have a modular help generator that can be configured and/or
authored by the users themselves as part of #2914, we will provide the
flexibility of turning off colored help messages but still wanting
colored error messages.

This flexibility was available before #2845 and @dbrgn immediately
noticed it and requested it back to which I agree. This was also
suggested by Josh in
[here](#2806 (comment))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

User confusion over help messages not being colored, despite color being enabled
3 participants