`multiple_occurrences` behaviour does not match documentation #2599

sstadick · 2021-07-15T14:43:26Z

Please complete the following tasks

I have searched the discussions
I have searched the existing issues

Clap Version

3.0.0-beta.2

Where?

https://docs.rs/clap/3.0.0-beta.2/clap/struct.Arg.html#method.multiple_occurrences

What's wrong?

The documentation claims that with the above method set to true, something like -a 1 2 would be disallowed and only -a 1 -a 2 would be allowed. However, the tests don't agree:

clap/clap_derive/tests/options.rs

Line 225 in 412b71e

assert_eq!(

And questions have been answered based on the docs: #2259 (comment)

How to fix?

Make behavior match the docs, unless there is some other way of getting the behavior described in the discussions link above.

To clarify further, I'd expect the test case:

    assert_eq!(
        Opt {
            arg: Some(vec![1, 2])
        },
        Opt::parse_from(&["test", "-a", "1", "2"])
    );

to instead be

    assert_eq!(
        Opt {
            arg: Some(vec![1])
        },
        Opt::parse_from(&["test", "-a", "1", "2"])
    );

The text was updated successfully, but these errors were encountered:

pksunkara · 2021-07-16T15:17:18Z

It's not a doc issue, but looks like a bug with how Vec adapter in clap_derive interacts with multiple* methods. Possibly a duplicate of #1772

ldm0 · 2021-07-20T18:05:15Z

Nope, I don't think it's a bug. Option<Vec<T>> implies multiple_values.

epage · 2021-07-21T20:40:00Z

When multiple was removed, Vec<_> was switched to multiple_values but I think what people would expect is that Vec<_> would map to multiple_occurrences.

I plan to bring this up in my work on #1772

pksunkara · 2021-07-25T14:11:10Z

Yeah, we need to allow combination of values, occurrences and both.

Before: - `bool`: a flag - `Option<_>`: not required - `Option<Option<_>>` is not required and when it is present, the value is not required - `Vec<_>`: multiple values, optional - `Option<Vec<_>>`: multiple values, min values of 1, optional After: - `bool`: a flag - `Option<_>`: not required - `Option<Option<_>>` is not required and when it is present, the value is not required - `Vec<_>`: multiple occurrences, optional - optional: `Vec` implies 0 or more, so should not imply required - `Option<Vec<_>>`: multiple occurrences, optional - optional: Use over `Vec` to detect when no option being present when using multiple values Motivations: My priorities were: 1. Are we getting in the users way? 2. Does the API make sense? 3. Does the API encourage best practices? I was originally concerned about the lack of composability with `Option<Option<_>>` and `Option<Vec<_>>` (and eventually `Vec<Vec<_>>`). It prescribes special meaning to each type depending on where it shows up, rather than providing a single meaning for a type generally. You then can't do things like have `Option<_>` mean "required argument with optional value" without hand constructing it. However, in practice the outer type correlates with the argument occurrence and the inner type with the value. It is rare to want the value behavior without also the occurrence behavior. So I figure it is probably fine as long as people can set the flags to manually get the behavior they want. `Vec<_>` implies multiple occurrences, rather than multiple values. Anecdotally, whenver I've used the old `Arg::multiple`, I thought I was getting `Arg::multiple_occurrences` only. `Arg::multiple_values`, without any bounds or delimeter requirement, can lead to a confusing user experience but isn't a good default for these. On top of that, if someone does have an unbounded or a delimeter multiple values, they are probably also using multiple occurrences. `Vec<_>` is optional because a `Vec` implies 0 or more, so we stick to the meaning of the rust type. `Option<Vec<_>>` ends up matching `Vec<_>` which an raise the question of why have it. Some users might prefer the type. Otherwise, this is so users can no when the argument is present or not when using `min_values(0)`. Rather than defining an entire policy around this and having users customize it, or setting `min_values(0)` without the rest of a default policy, this gives people a blank slate to work from. Another option would have been to not infer a setting if someone sets a handful of settings manually, which would have avoided the confusion in Issue clap-rs#2599 but I see that being confusing (for someone who knows the default, they will be expecting it to be additive; which flags?) and brittle (as flags are added or changed, how do we ensure we keep this up?) Tests were added to ensure we support people customizing the behavior to match their needs. This is not solving: - `Vec<Vec<_>>`, see clap-rs#2924 - `(T1, T2)`, `Vec<(T1, T2)>`, etc, see clap-rs#1717 Fixes clap-rs#1772 Fixes clap-rs#2599 See also clap-rs#2195

Before: - `bool`: a flag - `Option<_>`: not required - `Option<Option<_>>` is not required and when it is present, the value is not required - `Vec<_>`: multiple values, optional - `Option<Vec<_>>`: multiple values, min values of 0, optional After: - `bool`: a flag - `Option<_>`: not required - `Option<Option<_>>` is not required and when it is present, the value is not required - `Vec<_>`: multiple occurrences, optional - optional: `Vec` implies 0 or more, so should not imply required - `Option<Vec<_>>`: multiple occurrences, optional - optional: Use over `Vec` to detect when no option being present when using multiple values Motivations: My priorities were: 1. Are we getting in the users way? 2. Does the API make sense? 3. Does the API encourage best practices? I was originally concerned about the lack of composability with `Option<Option<_>>` and `Option<Vec<_>>` (and eventually `Vec<Vec<_>>`). It prescribes special meaning to each type depending on where it shows up, rather than providing a single meaning for a type generally. You then can't do things like have `Option<_>` mean "required argument with optional value" without hand constructing it. However, in practice the outer type correlates with the argument occurrence and the inner type with the value. It is rare to want the value behavior without also the occurrence behavior. So I figure it is probably fine as long as people can set the flags to manually get the behavior they want. `Vec<_>` implies multiple occurrences, rather than multiple values. Anecdotally, whenver I've used the old `Arg::multiple`, I thought I was getting `Arg::multiple_occurrences` only. `Arg::multiple_values`, without any bounds or delimeter requirement, can lead to a confusing user experience but isn't a good default for these. On top of that, if someone does have an unbounded or a delimeter multiple values, they are probably also using multiple occurrences. `Vec<_>` is optional because a `Vec` implies 0 or more, so we stick to the meaning of the rust type. `Option<Vec<_>>` ends up matching `Vec<_>` which an raise the question of why have it. Some users might prefer the type. Otherwise, this is so users can no when the argument is present or not when using `min_values(0)`. Rather than defining an entire policy around this and having users customize it, or setting `min_values(0)` without the rest of a default policy, this gives people a blank slate to work from. Another option would have been to not infer a setting if someone sets a handful of settings manually, which would have avoided the confusion in Issue clap-rs#2599 but I see that being confusing (for someone who knows the default, they will be expecting it to be additive; which flags?) and brittle (as flags are added or changed, how do we ensure we keep this up?) Tests were added to ensure we support people customizing the behavior to match their needs. This is not solving: - `Vec<Vec<_>>`, see clap-rs#2924 - `(T1, T2)`, `Vec<(T1, T2)>`, etc, see clap-rs#1717 Fixes clap-rs#1772 Fixes clap-rs#2599 See also clap-rs#2195

Before: - `bool`: a flag - `Option<_>`: not required - `Option<Option<_>>` is not required and when it is present, the value is not required - `Vec<_>`: multiple values, optional - `Option<Vec<_>>`: multiple values, min values of 0, optional After: - `bool`: a flag - `Option<_>`: not required - `Option<Option<_>>` is not required and when it is present, the value is not required - `Vec<_>`: multiple occurrences, optional - optional: `Vec` implies 0 or more, so should not imply required - `Option<Vec<_>>`: multiple occurrences, optional - optional: Use over `Vec` to detect when no option being present when using multiple values Motivations: My priorities were: 1. Are we getting in the users way? 2. Does the API make sense? 3. Does the API encourage best practices? I was originally concerned about the lack of composability with `Option<Option<_>>` and `Option<Vec<_>>` (and eventually `Vec<Vec<_>>`). It prescribes special meaning to each type depending on where it shows up, rather than providing a single meaning for a type generally. You then can't do things like have `Option<_>` mean "required argument with optional value" without hand constructing it. However, in practice the outer type correlates with the argument occurrence and the inner type with the value. It is rare to want the value behavior without also the occurrence behavior. So I figure it is probably fine as long as people can set the flags to manually get the behavior they want. `Vec<_>` implies multiple occurrences, rather than multiple values. Anecdotally, whenever I've used the old `Arg::multiple`, I thought I was getting `Arg::multiple_occurrences` only. `Arg::multiple_values`, without any bounds or delimiter requirement, can lead to a confusing user experience and isn't a good default for these. On top of that, if someone does have an unbounded or a delimiter multiple values, they are probably also using multiple occurrences. `Vec<_>` is optional because a `Vec` implies 0 or more, so we stick to the meaning of the rust type. `Option<Vec<_>>` ends up matching `Vec<_>` which an raise the question of why have it. Some users might prefer the type. Otherwise, this is so users can no when the argument is present or not when using `min_values(0)`. Rather than defining an entire policy around this and having users customize it, or setting `min_values(0)` without the rest of a default policy, this gives people a blank slate to work from. Another option would have been to not infer a setting if someone sets a handful of settings manually, which would have avoided the confusion in Issue clap-rs#2599 but I see that being confusing (for someone who knows the default, they will be expecting it to be additive; which flags?) and brittle (as flags are added or changed, how do we ensure we keep this up?) Tests were added to ensure we support people customizing the behavior to match their needs. This is not solving: - `Vec<Vec<_>>`, see clap-rs#2924 - `(T1, T2)`, `Vec<(T1, T2)>`, etc, see clap-rs#1717 Fixes clap-rs#1772 Fixes clap-rs#2599 See also clap-rs#2195

Before: - `bool`: a flag - `Option<_>`: not required - `Option<Option<_>>` is not required and when it is present, the value is not required - `Vec<_>`: multiple values, optional - `Option<Vec<_>>`: multiple values, min values of 0, optional After: - `bool`: a flag - `Option<_>`: not required - `Option<Option<_>>` is not required and when it is present, the value is not required - `Vec<_>`: multiple occurrences, optional - optional: `Vec` implies 0 or more, so should not imply required - `Option<Vec<_>>`: multiple occurrences, optional - optional: Use over `Vec` to detect when no option being present when using multiple values Motivations: My priorities were: 1. Are we getting in the users way? 2. Does the API make sense? 3. Does the API encourage best practices? I was originally concerned about the lack of composability with `Option<Option<_>>` and `Option<Vec<_>>` (and eventually `Vec<Vec<_>>`). It prescribes special meaning to each type depending on where it shows up, rather than providing a single meaning for a type generally. You then can't do things like have `Option<_>` mean "required argument with optional value" without hand constructing it. However, in practice the outer type correlates with the argument occurrence and the inner type with the value. It is rare to want the value behavior without also the occurrence behavior. So I figure it is probably fine as long as people can set the flags to manually get the behavior they want. `Vec<_>` implies multiple occurrences, rather than multiple values. Anecdotally, whenever I've used the old `Arg::multiple`, I thought I was getting `Arg::multiple_occurrences` only. `Arg::multiple_values`, without any bounds or delimiter requirement, can lead to a confusing user experience and isn't a good default for these. On top of that, if someone does have an unbounded or a delimiter multiple values, they are probably also using multiple occurrences. `Vec<_>` is optional because a `Vec` implies 0 or more, so we stick to the meaning of the rust type. At least for me, I also rarely need a required with multiple occurrences argument but more often need optional with multiple occurrences. `Option<Vec<_>>` ends up matching `Vec<_>` which an raise the question of why have it. Some users might prefer the type. Otherwise, this is so users can no when the argument is present or not when using `min_values(0)`. Rather than defining an entire policy around this and having users customize it, or setting `min_values(0)` without the rest of a default policy, this gives people a blank slate to work from. Another option would have been to not infer a setting if someone sets a handful of settings manually, which would have avoided the confusion in Issue clap-rs#2599 but I see that being confusing (for someone who knows the default, they will be expecting it to be additive; which flags?) and brittle (as flags are added or changed, how do we ensure we keep this up?) Tests were added to ensure we support people customizing the behavior to match their needs. This is not solving: - `Vec<Vec<_>>`, see clap-rs#2924 - `(T1, T2)`, `Vec<(T1, T2)>`, etc, see clap-rs#1717 Fixes clap-rs#1772 Fixes clap-rs#2599 See also clap-rs#2195

Before: - `bool`: a flag - `Option<_>`: not required - `Option<Option<_>>` is not required and when it is present, the value is not required - `Vec<_>`: multiple values, optional - `Option<Vec<_>>`: multiple values, min values of 0, optional After: - `bool`: a flag - `Option<_>`: not required - `Option<Option<_>>` is not required and when it is present, the value is not required - `Vec<_>`: multiple occurrences, optional - optional: `Vec` implies 0 or more, so should not imply required - `Option<Vec<_>>`: multiple occurrences, optional - optional: Use over `Vec` to detect when no option being present when using multiple values Motivations: My priorities were: 1. Are we getting in the users way? 2. Does the API make sense? 3. Does the API encourage best practices? I was originally concerned about the lack of composability with `Option<Option<_>>` and `Option<Vec<_>>` (and eventually `Vec<Vec<_>>`). It prescribes special meaning to each type depending on where it shows up, rather than providing a single meaning for a type generally. You then can't do things like have `Option<_>` mean "required argument with optional value" without hand constructing it. However, in practice the outer type correlates with the argument occurrence and the inner type with the value. It is rare to want the value behavior without also the occurrence behavior. So I figure it is probably fine as long as people can set the flags to manually get the behavior they want. `Vec<_>` implies multiple occurrences, rather than multiple values. Anecdotally, whenever I've used the old `Arg::multiple`, I thought I was getting `Arg::multiple_occurrences` only. `Arg::multiple_values`, without any bounds or delimiter requirement, can lead to a confusing user experience and isn't a good default for these. On top of that, if someone does have an unbounded or a delimiter multiple values, they are probably also using multiple occurrences. `Vec<_>` is optional because a `Vec` implies 0 or more, so we stick to the meaning of the rust type. At least for me, I also rarely need a required with multiple occurrences argument but more often need optional with multiple occurrences. `Option<Vec<_>>` ends up matching `Vec<_>` which can raise the question of why have it. Some users might prefer the type. Otherwise, this is so users can detect whether the argument is present or not when using `min_values(0)`. Rather than defining an entire policy around this and having users customize it, or setting `min_values(0)` without the rest of a default policy, this gives people a blank slate to work from. Another design option would have been to not infer any special-type settings if someone sets a handful of settings manually, which would have avoided the confusion in Issue clap-rs#2599 but I see that being confusing (for someone who knows the default, they will be expecting it to be additive; which flags disable inferred settings?) and brittle (as flags are added or changed, how do we ensure we keep this up?). Tests were added to ensure we support people customizing the behavior to match their needs. This is not solving: - `Vec<Vec<_>>`, see clap-rs#2924 - `(T1, T2)`, `Vec<(T1, T2)>`, etc, see clap-rs#1717 - `Vec<Option<_>>` and many other potential combinations Fixes clap-rs#1772 Fixes clap-rs#2599 See also clap-rs#2195

epage · 2021-12-07T21:58:23Z

Something like #2993 was recently merged, along with a new derive reference.

sstadick added the A-docs Area: documentation, including docs.rs, readme, examples, etc... label Jul 15, 2021

pksunkara added C-bug Category: Updating dependencies and removed A-docs Area: documentation, including docs.rs, readme, examples, etc... labels Jul 16, 2021

pksunkara added this to the 3.0 milestone Jul 16, 2021

epage mentioned this issue Jul 22, 2021

clap_derive: Vec/Option<Vec> behavior is inconsistent with other types #1772

Closed

pksunkara added the A-derive Area: #[derive]` macro API label Aug 13, 2021

epage mentioned this issue Nov 5, 2021

fix(derive): Define multiple policy for Special Types #2993

Closed

Ten0 mentioned this issue Nov 14, 2021

Incorrect parsing - The subcommand 'bar' wasn't recognized, did you mean 'bar'? #3024

Closed

2 tasks

epage mentioned this issue Dec 6, 2021

multiple_occurrences behaviour does not match documentation epage/clapng#190

Open

2 tasks

epage closed this as completed Dec 7, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`multiple_occurrences` behaviour does not match documentation #2599

`multiple_occurrences` behaviour does not match documentation #2599

sstadick commented Jul 15, 2021 •

edited

Loading

pksunkara commented Jul 16, 2021

ldm0 commented Jul 20, 2021

epage commented Jul 21, 2021

pksunkara commented Jul 25, 2021

epage commented Dec 7, 2021

multiple_occurrences behaviour does not match documentation #2599

multiple_occurrences behaviour does not match documentation #2599

Comments

sstadick commented Jul 15, 2021 • edited Loading

Please complete the following tasks

Clap Version

Where?

What's wrong?

How to fix?

pksunkara commented Jul 16, 2021

ldm0 commented Jul 20, 2021

epage commented Jul 21, 2021

pksunkara commented Jul 25, 2021

epage commented Dec 7, 2021

`multiple_occurrences` behaviour does not match documentation #2599

`multiple_occurrences` behaviour does not match documentation #2599

sstadick commented Jul 15, 2021 •

edited

Loading