clap icon indicating copy to clipboard operation
clap copied to clipboard

Show the type of arguments in help message?

Open epage opened this issue 1 year ago • 7 comments

Discussed in https://github.com/clap-rs/clap/discussions/5153

Originally posted by tigerros October 1, 2023 Let's say you have this:

#[derive(Parser, Debug)]
#[command(author, version, about, long_about = None)]
pub struct MyParser {
    #[arg(long)]
    pub foo: u32,
}

Running --help will show this message:

Options:
      --foo <FOO>
  -h, --help       Print help
  -V, --version    Print version

Is it possible to also show the type? For example:

--foo <FOO> (unsigned 32 bit integer)

u32 would be nice, but it's not standard.

epage avatar Oct 02 '23 17:10 epage

There is an argument parser for another language that does this; I wish I could remember which.

If we did this, it could work to fill in similar to [possible values: foo, bar] in style.

However, I'm concerned about

* Clutter in the help output

* Not being in the user's terms (while most CLI users are programmers, not all are), even if its "unsigned 32 bit integer".

However, the value_name fills a similar role and I think it'd be a worthwhile to look into leveraging that. long and id are more akin to a field name while value_name is more akin to the type (in flags, positionals are weird in that they need to convey both). Framing this way, makes me feel like we could have a type associated term as the value name.

I had considered such a thing in #2683 but held off in #3732.

epage avatar Oct 02 '23 17:10 epage

My thought

  • Add to TypedValueParser a fn value_name(&self) -> String with an inherent impl that returns VALUE
    • Update various built-in value parsers to return a meaningful name
    • Add to TypedValueParser a fn deferred_name(&self, name: impl Fn() -> String) -> NamedValueParser so it can be added to closures, etc
    • Adjust things so deriving ValueEnum will also allow setting deferred_name, with the default being the type's name, turned into SCREAMING_CASE
    • Add a setting to some value parsers so users can decide between showing the values vs name
  • Update --help to show the value parser's value_name for flags
    • Is there anything we can do for positionals?
    • Is the divergence between named and positional values worth it?

So this would look like

  • --option <BOOL> (placeholder) or --option <true|false> (literal
  • --option <PATH>
    • clio could then specialize this further as --option <FILE> and --option <DIR>
  • --option <NUM> (placeholder) or --option <10..30> (literal)
  • --option <MY_ENUM> (placeholder) or --option <foo|bar> (literal)

epage avatar Oct 02 '23 17:10 epage

  • Clutter in the help output
  • Not being in the user's terms (while most CLI users are programmers, not all are), even if its "unsigned 32 bit integer".

By making it optional both of these issues are solved, no?

tigerros avatar Oct 02 '23 18:10 tigerros

In clap, we have to balance features with compile time, binary size, and API size. Adding the suggested solution for a niche would hurt the whole. If you are set on it being implemented that way, you can extend your doc comments to include the type information.

epage avatar Oct 02 '23 20:10 epage

We talked about this some in the WG-CLI meeting

We'd like to keep configuration down to a minimum due to compile time + binary size concerns. This means we'd likely not want to make placerholder vs literal configurable. In that case, we leaned towards literals (when a meaningful enough one can be made). If we default to literals, then working around it with value_name("10..30") requires them to duplicate information they provide to clap. On the other hand, if we default to literals, then the work around is to do value_name("NUM") which is a unique textual representation.

epage avatar Nov 06 '23 19:11 epage

Current proposal

Add:

trait TypedValueParser {
    // ... existing

    /// Fallback for [`Arg::value_name`] for options
    fn get_value_name(&self) -> Option<String> {
        None
    }

    /// Fallback for [`Arg::value_name`] for options
    fn value_parser(self, name: fn() -> String) -> NamedValueParser {
        NamedValueParser { name, self }
    }
}

pub NamedValueParser {
    name: fn() -> String,
    inner: impl TypedValueParser
}

// ....

let value_names = if let Some(value_names) = arg.get_value_names() {
    value_names
} else if arg.is_positional() {
    [arg.get_id()]
} else {
    self.get_value_parser().get_value_names()
};

Hmm, the main problem with this is this mostly helps clap_derive and clap_derive implicitly calls Arg::value_name. So we'd need to decide if we wanted to make that conditional or not.

epage avatar Nov 06 '23 19:11 epage

Just realized that a way to force showing of the literal would be to respect hide_possible_values. This would help cases like #5203.

epage avatar Nov 09 '23 16:11 epage