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

clap_derive: Default value support for Vec of ValueEnums #3932

Closed
2 tasks done
emersonford opened this issue Jul 14, 2022 · 2 comments
Closed
2 tasks done

clap_derive: Default value support for Vec of ValueEnums #3932

emersonford opened this issue Jul 14, 2022 · 2 comments
Labels
C-enhancement Category: Raise on the bar on expectations

Comments

@emersonford
Copy link
Contributor

emersonford commented Jul 14, 2022

Please complete the following tasks

Clap Version

3.2.10

Describe your use case

Suppose you have the following:

#[derive(clap::ValueEnum, PartialEq, Debug, Clone)]
enum ArgChoice {
    Foo,
    Bar,
}

#[derive(Parser, PartialEq, Debug)]
struct Opt {
    #[clap(value_enum, value_parser)]
    arg: Vec<ArgChoice>,
}

providing default values for arg doesn't feel very ergonomic to me. As far as I'm aware, you have the following options:

  1. #[clap(value_enum, value_parser, default_value="foo,bar", value_delimiter=',')] as shown in fix: multiple default values combined with ArgEnum / PossibleValues #3541
    • Doesn't feel super great to me since you're forced into using value_delimiter and can't easily set the default to "all variants". Also, if you ever use rename_all, you'd have to ensure default_value also gets changed.
  2. default_values(...) with something like
    ArgChoice::value_variants().iter().map(|val| val.to_possible_value().unwrap().get_name()).collect::<Vec<_>>().as_slice()
    or
    &[ArgChoice::Foo.to_possible_value().unwrap().get_name(), ArgChoice::Foo.to_possible_value().unwrap().get_name()]
    • Requires a lot of boiler plate and repeated code.

Describe the solution you'd like

Fixing default_value_t so you can do the following:

#[derive(clap::ValueEnum, PartialEq, Debug, Clone)]
enum ArgChoice {
    Foo,
    Bar,
}

#[derive(Parser, PartialEq, Debug)]
struct Opt {
    #[clap(value_enum, value_parser, default_value_t = vec![ArgChoice::Foo])]
    arg: Vec<ArgChoice>,
}

and

#[derive(clap::ValueEnum, PartialEq, Debug, Clone)]
enum ArgChoice {
    Foo,
    Bar,
}

#[derive(Parser, PartialEq, Debug)]
struct Opt {
    #[clap(value_enum, value_parser, default_value_t = ArgChoice::value_variants().to_vec())]
    arg: Vec<ArgChoice>,
}

Alternatives, if applicable

I could see an argument for adding a default_values_t attr, but since default_value_t doesn't work to begin with for Vec<impl ValueEnum>, I feel like we might as well fix default_value_t (plus I personally find it more intuitive, if I used the "typed" version for a Vec, I should be able to pass a Vec).

Additional Context

default_value_t doesn't work in any way:

#[derive(Parser, PartialEq, Debug)]
struct Opt {
    #[clap(
        value_enum, 
        value_parser, 
        default_value_t = ArgChoice::Foo,
    )]
    arg: Vec<ArgChoice>,
}
error[[E0308]](https://doc.rust-lang.org/stable/error-index.html#E0308): mismatched types
  --> src/main.rs:14:27
   |
14 |         default_value_t = ArgChoice::Foo,
   |                           ^^^^^^^^^^^^^^ expected struct `Vec`, found enum `ArgChoice`
15 |     )]
16 |     arg: Vec<ArgChoice>,
   |          -------------- expected due to this
   |
   = note: expected struct `Vec<ArgChoice>`
                found enum `ArgChoice`
#[derive(Parser, PartialEq, Debug)]
struct Opt {
    #[clap(
        value_enum, 
        value_parser, 
        default_value_t = vec![ArgChoice::Foo],
    )]
    arg: Vec<ArgChoice>,
}
error[[E0277]](https://doc.rust-lang.org/stable/error-index.html#E0277): the trait bound `Vec<ArgChoice>: ArgEnum` is not satisfied
  --> src/main.rs:14:9
   |
14 |         default_value_t = vec![ArgChoice::Foo],
   |         ^^^^^^^^^^^^^^^ the trait `ArgEnum` is not implemented for `Vec<ArgChoice>`
   |
   = help: the trait `ArgEnum` is implemented for `ArgChoice`

so changing default_value_t's behavior here for a Vec of ValueEnums wouldn't be a breaking change.

@emersonford
Copy link
Contributor Author

documenting this here from the PR:

I realized I could apply this for all Vec, not just a Vec.

so I added another commit to the PR to fix default_value_t for all Vec types

@benny-n
Copy link

benny-n commented Jul 14, 2022

I realized that I need this feature today and this design is exactly what I had in mind!
Looking forward to seeing this merged 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: Raise on the bar on expectations
Projects
None yet
Development

No branches or pull requests

2 participants