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

Ability to show possible value help in --help #3312

Closed
2 tasks done
kpreid opened this issue Jan 19, 2022 · 20 comments
Closed
2 tasks done

Ability to show possible value help in --help #3312

kpreid opened this issue Jan 19, 2022 · 20 comments
Labels
A-help Area: documentation, including docs.rs, readme, examples, etc... C-enhancement Category: Raise on the bar on expectations S-waiting-on-decision Status: Waiting on a go/no-go before implementing
Milestone

Comments

@kpreid
Copy link
Contributor

kpreid commented Jan 19, 2022

Please complete the following tasks

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

Clap Version

3.0.8

Describe your use case

I have options which take one of several possible values and which would benefit from explaining what each of the values do. I am currently putting this text in the option's help but I could easily forget to update that, and a list inside the help text doesn't wrap nicely.

Describe the solution you'd like

A setting which causes each possible value's PossibleValue::help to be displayed as part of the help for the option (instead of just listing the possible values without any help, which is the current default behavior).

(Perhaps it should not be an additional setting, but be the default behavior whenever the possible values have help strings. I don't know what typical use cases look like to say whether this would be undesirable verbosity.)

Alternatives, if applicable

I currently have hardcoded option help text listing the values, which is what I was hoping PossibleValue::help would replace before I tried it and found that it didn't. Another alternative would be to implement this using existing facilities by iterating over the possible values and building a help string. However, clap could do this better than the application can, by formatting the list of values and help in a text-wrapping-aware way.

Additional Context

No response

@kpreid kpreid added the C-enhancement Category: Raise on the bar on expectations label Jan 19, 2022
@epage
Copy link
Member

epage commented Jan 19, 2022

@ModProg as the person who implemented this, I assume you use this and would have thoughts / perspective on whether we should show this in --help, whether always or behind a flag.

If we do show the PossibleValue::help, we would probably limit it to --help (long help) and not show it in -h (short help)

@ModProg
Copy link
Contributor

ModProg commented Jan 19, 2022

If we do show the PossibleValue::help, we would probably limit it to --help (long help) and not show it in -h (short help)

Yeah, that makes sense I'd say, should probably be configurable, but the current behavior of only showing the help in the completions seams a bit strange in retrospect. Should have been in the help as well, is my gut reaction.

@ModProg
Copy link
Contributor

ModProg commented Jan 19, 2022

But I'm not sure on what would be the best way of displaying this:

    -p, --pos <VAL>      Some vals [possible values: fast, slow]
    -p, --pos <VAL>      Some vals
                         possible values: 
                            fast 
                            slow (not as fast)

@epage
Copy link
Member

epage commented Jan 19, 2022

Yes, I was thinking a bulleted list would probably be how we show this. iirc I brought up showing them this way in the PR for clap_man.

The next question is if this would be considered a "breaking change", meaning that people have crafted their CLIs around the current behavior and it would egregiously impact the UI to change this without a more explicit acknowledgement of breaking behavior.

For builder users, it depends on if they've discovered and started to use this new feature while manually documenting the value meanings.

For derive users, the situation is different because we are picking up the doc comments and users are most likely not using clap_complete and if they are, they are unlikely to notice the value descriptions. We could start showing to users comments meant for developers.

@ModProg
Copy link
Contributor

ModProg commented Jan 19, 2022

Well not if it was off by default, but I see your point.

As I would like this to be the default behaviour as that seams like the natural effect of the help() IMO.

This could be done in a two step prosses I guess (first opt in, then opt out)? Not sure, what the best way is.

@epage
Copy link
Member

epage commented Jan 19, 2022

My preference is to avoid configuration long term. Clap's API is so bloated, it makes it difficult to know whats there or not. If we can make a policy that can reasonably apply to nearly all programs, we have issues like #2914 for the rest.

If we are concerned about the transition, we could use either a manifest feature flag or a setting as we transition to the new behavior. maybe we even toggle it for the major release after that but the following would see the option removed ideally.

@ModProg
Copy link
Contributor

ModProg commented Jan 19, 2022

yeah, I think this is the expected behavior, I just did not think about it when I first started looking into this.

IMO as the doc comments are relevant on all other clap derives, it is reasonable to have them just appear in the help message.

We can put a warning in the change log (tho I understand that most people won't look there).

@epage
Copy link
Member

epage commented Jan 19, 2022

We can put a warning in the change log (tho I understand that most people won't look there).

Yeah, another option I was considering was waiting for 3.1 and having a "Backwards Compatibility" section in the changelog, like Rust.

@ModProg
Copy link
Contributor

ModProg commented Jan 19, 2022

We can put a warning in the change log (tho I understand that most people won't look there).

Yeah, another option I was considering was waiting for 3.1 and having a "Backwards Compatibility" section in the changelog, like Rust.

Sounds reasonable, if there are more features like this that are kind of unstable/breaking changes we could put them behind a feature flag until 3.1 or just have an off by default setting till then.

@kpreid
Copy link
Contributor Author

kpreid commented Jan 19, 2022

Some thoughts as the requester:

I hadn't thought of the documentation not being suitable to be shown, but I was thinking that it might be too long. However, long possible value lists are likely to be hide_possible_values already, and the documentation for PossibleValue::help already says it should be concise, so that shouldn't be a problem.

Some command line tools offer dedicated options to list possible values when there are lots of them (to name a couple of examples I've seen, lists of compatible hardware, or file formats/codecs); it might be interesting to offer help implementing that, but on the other hand it's easy to do and might want customization.

As to specifics of formatting, here is the result of what I picked for my application-specific implementation:

    -g, --graphics <MODE>        Graphics/UI mode; one of the following keywords:
                                 • window   — Open a window (uses OpenGL)
                                 • terminal — Colored text in this terminal (uses raytracing)
                                 • headless — Non-interactive; don't draw anything but only
                                 simulates
                                 • record   — Non-interactive; save an image or video (uses
                                 raytracing)
                                 • print    — Non-interactive; print one frame like 'terminal'
                                 mode then exit
                                  [default: window]

I don't entirely recommend exactly this: the bullets are a bit noisy, but I added them because from the outside I can't get clap to indented-wrap the entries for visual clarity (that is, indent the wrapped lines "simulates" and "mode then exit" so they are visibly part of the preceding item rather than their own items), and even if I did my own text wrapping I wouldn't know how long the left option-name column is.

I do think the columnar alignment is a good idea given enough available width. Here's my implementation:

static GRAPHICS_HELP: Lazy<String> = Lazy::new(|| {
    let pv_iter = GraphicsType::value_variants()
        .iter()
        .filter_map(|v| v.to_possible_value());

    let max_width = pv_iter
        .clone()
        .filter_map(|pv| pv.get_visible_name())
        .map(str::len)
        .max()
        .unwrap_or(0);

    let mut text = String::from("Graphics/UI mode; one of the following keywords:\n");
    for pv in pv_iter {
        // Note: There's a final newline so that clap's default value text is put on a new line.
        writeln!(
            text,
            "• {:max_width$} — {}",
            pv.get_name(),
            pv.get_help().unwrap()
        )
        .unwrap();
    }
    text
});

(This isn't quite fully general: it doesn't handle hidden items or ones without documentation, which didn't matter since it's hardcoded to this one enum.)

@epage
Copy link
Member

epage commented Jan 19, 2022

I hadn't thought of the documentation not being suitable to be shown,

In case it wasn't clear, the concern is more with people who have already written rustdoc comments who might be surprised to see those starting to show up in the help

I hadn't thought of the documentation not being suitable to be shown, but I was thinking that it might be too long. However, long possible value lists are likely to be hide_possible_values already, and the documentation for PossibleValue::help already says it should be concise, so that shouldn't be a problem.

Yes, that is a concern. One thing that helps is that clap has distinct -h and --help. -h is meant to be brief and --help is meant to go into all of the gory details. -h would still just show the list of possible values and we'd only be changing --help.

Some command line tools offer dedicated options to list possible values when there are lots of them (to name a couple of examples I've seen, lists of compatible hardware, or file formats/codecs); it might be interesting to offer help implementing that, but on the other hand it's easy to do and might want customization.

A good example of this (though its more dynamic)

$ cargo test --test    
error: "--test" takes one argument.                                      
Available tests:                    
    builder     
    derive                                                               
    derive_ui                       
    examples                                                             
    macros                                                                                                                                         
    yaml                                                                 

I feel like we could model our errors off of this. Right now, we don't use possible values in the errors:

$ git-stack --color
    Finished dev [unoptimized + debuginfo] target(s) in 0.04s
     Running `/home/epage/src/personal/git-stack/target/debug/git-stack --color`
error: The argument '--color <WHEN>' requires a value but none was supplied

USAGE:
    git-stack [OPTIONS]

For more information try --help

$ git-stack --color lklk
    Finished dev [unoptimized + debuginfo] target(s) in 0.04s
     Running `/home/epage/src/personal/git-stack/target/debug/git-stack --color lklk`
error: Invalid value for '--color <WHEN>': Unknown color choice 'lklk'

For more information try --help

As to specifics of formatting, here is the result of what I picked for my application-specific implementation:

If we only do this for --help, we'll also be using NextLineHelp which will buy us some space (you can do this today with long_help()).

It is a good point that we should indent any wrapped lines for the bullets.

As for the exact bullet symbol, we can determine that later.

Using coloring will also help.

@epage epage added A-help Area: documentation, including docs.rs, readme, examples, etc... S-waiting-on-decision Status: Waiting on a go/no-go before implementing labels Jan 27, 2022
@epage epage modified the milestones: 3.1, 4.0 Feb 2, 2022
epage added a commit to epage/clap that referenced this issue Feb 11, 2022
Like was said in clap-rs#2435, this is what people would expect.

While we should note this in a compatibility section in the changelog, I
do not consider this a breaking change since we should be free to adjust
the help output as needed.  We are cautious when people might build up
their own content around it (like clap-rs#3312) but apps should already handle
this with `--help` so this shouldn't be a major change.

We aren't offering a way for people to disable this, assuming people
won't need to.  Longer term, we are looking at support "Actions" (clap-rs#3405)
and expect those to help customize the flags.  We'll need something
similar for the `help` subcommand.

Fixes clap-rs#3440
epage added a commit to epage/clap that referenced this issue Feb 11, 2022
Like was said in clap-rs#2435, this is what people would expect.

While we should note this in a compatibility section in the changelog, I
do not consider this a breaking change since we should be free to adjust
the help output as needed.  We are cautious when people might build up
their own content around it (like clap-rs#3312) but apps should already handle
this with `--help` so this shouldn't be a major change.

We aren't offering a way for people to disable this, assuming people
won't need to.  Longer term, we are looking at support "Actions" (clap-rs#3405)
and expect those to help customize the flags.  We'll need something
similar for the `help` subcommand.

Fixes clap-rs#3440
@ModProg
Copy link
Contributor

ModProg commented Feb 16, 2022

Just ran into this exact issue.

I now needed to specify separate help messages for long and short, because I wanted to have [possible values ...] in short help but

- a help message
- b help message

in long help.

I thought about adding an aditional hide_on_short/long_help to hide_possible_values, but that would only treat the symptom of not being able to have help messages on possible values.

If you have a plan on how this should be implemented @epage I would go ahead and look into it.

@epage
Copy link
Member

epage commented Feb 16, 2022

If you have a plan on how this should be implemented @epage I would go ahead and look into it.

I've not done this yet because I keep oscillating on whether this would be considered a breaking change or not, with the biggest concern being for derive users.

What we can do is add a unstable-v4 feature flag and guard the logic switch and tests with that flag.

Steps

  • Add unstable-v4 feature flag to Cargo.toml and document it in the README
  • Add a _FEATURES_next to the Makefile which is _FEATURES_full + unstable-v4
  • When outputting possible values, use the bulleted list when if printing long and possible.any(|p| p.get_about().is_some())

Rough template

<arg.get_long_about()>

Possible values:
<for possible in possibles>
- {GOOD}<possible.get_name()>{/GOOD}: <possible.get_help()>
</for>

(of course, willing to hear other ideas on any of this)

@ModProg
Copy link
Contributor

ModProg commented Feb 19, 2022

Makes sense, if I see correctly, the new possible value display would need to go here:

as the other possible_values are contained in a str and cannot have formatting.

Should extract the logic whether to use the long possible value help in a helper function so that hiding the old help and showing the new help use the same code?

@epage
Copy link
Member

epage commented Feb 20, 2022

Yes, we'll have possible-value printing in two different places, depending on if its long or short.

That is the place for doing it for long. The current place we print possible values is exclusively for the [] hints and so it wouldn't quite fit for us to output a long list in there, so we should specialize that for short possible values.

@ModProg
Copy link
Contributor

ModProg commented Feb 20, 2022

After some more investigation, a few questions:

  1. Is there a case where long help is printed without next-line?
  2. Should possible value help wrap like the normal help does?
    a. Print behind the PossibleValue::name as long as they fit in line, aligned by the longest possible value.
    b. Wrap when they are too long.
    c. When one of the PossibelValue::names surpasses a threshold, they are displayed in the next line. Indented by one TAB.

@epage
Copy link
Member

epage commented Feb 21, 2022

Is there a case where long help is printed without next-line?

Most likely not but unsure why this is relevant. We should be basing what we do off of self.use_long.

Should possible value help wrap like the normal help does?

Ideally

@ModProg
Copy link
Contributor

ModProg commented Feb 22, 2022

Most likely not but unsure why this is relevant.

It is relevant because it means that the indenting logic needs to support it.

@ModProg
Copy link
Contributor

ModProg commented Feb 22, 2022

The possible value name will not be wrapped (when using wrap_help), this is the same as for too long argument flags. This is porbably not relevant for a normal usecase, but I thought I'd mention it.

@epage
Copy link
Member

epage commented Mar 2, 2022

This is now available in v3.1.4 behind the unstable-v4 feature flag.

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-waiting-on-decision Status: Waiting on a go/no-go before implementing
Projects
None yet
Development

No branches or pull requests

3 participants