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

feat(AppSettings): Enable user-defined colors for help message #3407

Closed
wants to merge 5 commits into from

Conversation

codeinred
Copy link

Introduction

This pull request adds support for configuring colors and styles in Clap. It does this by adding a StyleSpec class that holds information about the termcolor::ColorSpecs associated with various styles. This is added as a component of the App class, and it allows users to set colors, styles, and other such aspects of Clap.

For example, I can now write the following:

use clap::{Color, IntoApp, Parser, Style};

fn main() {
    let app = Args::into_app()
        .bold(Style::Good, true)
        .bold(Style::Warning, true)
        .foreground(Style::Warning, Some(Color::Green));

    // ...

In this example, this will set text displayed in Style::Good and Style::Warning to bold, and also update the warning style color to Green.

Before Customization:
image

After Customization:
image

Motivation

I wanted to be able to make help flags and section headers display in bold for greater readability. Adding support for the ability to customize that involved a rework of a lot of Colorizer code, and it wasn't much additional work to enable other customizations as well.

Changes to clap's API

This section documents changes to the public API of clap

Newly exposed types:

  • Expose clap::output::fmt::Style as clap::Style
  • Expose termcolor::Color as clap::Color
  • Expose termcolor::ColorSpec as clap::ColorSpec

Additions to App

The following functions were added to App, to enable users to customize colors and styles:

pub fn style(mut self, style: Style, spec: termcolor::ColorSpec) -> Self;
pub fn bold(mut self, style: Style, value: bool) -> Self;
pub fn italic(mut self, style: Style, value: bool) -> Self;
pub fn underline(mut self, style: Style, value: bool) -> Self;
pub fn dimmed(mut self, style: Style, value: bool) -> Self;
pub fn intense(mut self, style: Style, value: bool) -> Self;
pub fn foreground(mut self, style: Style, color: Option<Color>) -> Self;
pub fn background(mut self, style: Style, color: Option<Color>) -> Self;

A note on the Style enum

This pull request functions within the bounds of the Style enum already defined by Clap, inside src/output/fmt.rs. We may want to give the options defined in this enum more descriptive names (for example, Style::Warning would become Style::SectionHeader, which is what it's used for when printing --help). We may also want to increase the number of styles (so that there's a Warning style and a SectionHeader style)

Internal additions / changes

  • Create StyleSpec, which holds a ColorSpec for each Style
  • Make StyleSpec a parameter of Colorizer::new
  • Update all uses of Colorizer::new to include a StyleSpec

Related issues and discussions

Issues:

Discussions:

The following changes were made as part of this commit:
- Make StyleSpec a parameter of Colorizer::new
- Enable setting ColorSpecs for particular styles
- Add Style as part of the public interface of Clap
- Add convinience methods to apply styles bold, italic, etc
- Add support for setting foreground and background colors

Newly exposed types:
- Expose clap::output::fmt::Style as clap::Style
- Expose termcolor::Color as clap::Color
- Expose termcolor::ColorSpec as clap::ColorSpec

Signed-off-by: Alecto Irene Perez <[email protected]>
This commit fixes the added color customization code so that it's
properly disabled if the color feature is missing.

It also updates the debug macro to use the updated signature for
Colorizer::new.

Signed-off-by: Alecto Irene Perez <[email protected]>
Signed-off-by: Alecto Irene Perez <[email protected]>
This cleans up the implementation and usage of StyleSpec by making it
indexable on the Style enum. The usage ends up being a lot cleaner.

Signed-off-by: Alecto Irene Perez <[email protected]>
@epage
Copy link
Member

epage commented Feb 5, 2022

Thank you for your interest in solving this but we have previously rejected extending the API like this (#3234). I'd recommend starting new design discussions in issues rather than PRs to help catch things like this before putting in all of this effort.

Other high level thoughts just so you have an idea on how we think about these things for future contributions

  • We have to worry about API stability and the current styles are not ready to be exposed. API stability isn't just about what names are but expectations for how they are used. We can make it so we can add more entries to the enum but dramatically changing how they apply will break user expectations.
  • This API feels backwards. an app shouldn't be responsible for applying formatting to styles in the style sheet. Instead the user should be defining the formatting of a style and passing that in, overriding the built-in style.

@epage epage closed this Feb 5, 2022
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.

2 participants