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

Can't use ValueParser to replace PossibleValues in v4 #4351

Closed
2 tasks done
o2sh opened this issue Oct 5, 2022 · 5 comments · Fixed by #4352
Closed
2 tasks done

Can't use ValueParser to replace PossibleValues in v4 #4351

o2sh opened this issue Oct 5, 2022 · 5 comments · Fixed by #4352
Labels
A-docs Area: documentation, including docs.rs, readme, examples, etc... C-bug Category: Updating dependencies

Comments

@o2sh
Copy link

o2sh commented Oct 5, 2022

Please complete the following tasks

Rust Version

1.64.0

Clap Version

4.08

Minimal reproducible code

static COLOR_RESOLUTIONS: [&str; 5] = ["16", "32", "64", "128", "256"];

#[arg(
    long,
    value_name = "VALUE",
    requires = "image",
    default_value_t = 16usize,
    // possible_values = ["16", "32", "64", "128", "256"], // Used to work before v4
    value_parser = PossibleValuesParser::new(COLOR_RESOLUTIONS) // fails
)]
pub color_resolution: usize,
$ cargo run

thread 'main' panicked at 'Mismatch between definition and access of `color_resolution`. Could not downcast to TypeId { t: 13834754221672687376 }, need to downcast to TypeId { t: 5005868189860995316 }
@o2sh o2sh added the C-bug Category: Updating dependencies label Oct 5, 2022
@epage epage added the A-docs Area: documentation, including docs.rs, readme, examples, etc... label Oct 5, 2022
@epage
Copy link
Member

epage commented Oct 5, 2022

For now, PossibleValuesParser only works with String though you can use TypedValueParser::map to adapt the type. In #4352, I'm adding some examples on how to do this.

messense added a commit to rust-cross/cargo-xwin that referenced this issue Oct 8, 2022
@aldanor
Copy link

aldanor commented Oct 26, 2022

For now, PossibleValuesParser only works with String

Just wondering, is it planned to have it back working with other types eventually? (e.g. that implement FromStr?)

Chances are, these lines will be copied/pasted many-many times all over the place (it's just one of the most common use cases for the old possible_values):

use clap::builder::{PossibleValuesParser, TypedValueParser as _};
...
    value_parser = PossibleValuesParser::new(Foo::VARIANTS)
        .map(|s| s.parse::<Foo>().unwrap())

E.g.:

    value_parser = PossibleValuesFromStr::<Foo>::new(Foo::VARIANTS)

@epage
Copy link
Member

epage commented Oct 26, 2022

Feel free to create an issue for making it more convenient.

@aldanor
Copy link

aldanor commented Oct 26, 2022

@epage An issue or a PR? If you have a particular view on what would be the most convenient option(s) I wouldn't mind contributing.

(e.g. bringing back something along the lines of possible_values_from_str = [...] as probably the most convenient thing, with the name being more explicit so as to be less confusing; or one step back, PossibleValuesFromStr as above).

@epage
Copy link
Member

epage commented Oct 26, 2022

Issue first as I prefer exploring on directions there. Once we have a direction, you are welcome to get a PR going.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-docs Area: documentation, including docs.rs, readme, examples, etc... C-bug Category: Updating dependencies
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants