arg_from_usage(): required values not being enforced when followed by another option

[ajdlinux]

I've got a SubCommand defined with a few options that have required values:

SubCommand::with_name("format")
    .about("Prepare patch series for email")
    .arg_from_usage("--in-reply-to [Message-Id] 'Make the first mail a reply to the specified Message-Id'")
    .arg_from_usage("--no-from 'Don't include in-body \"From:\" headers when formatting patches authored by others'")
    .arg_from_usage("-v, --reroll-count=[N] 'Mark the patch series as PATCH vN'")
    .arg_from_usage("--stdout 'Write patches to stdout rather than files'")
    .arg_from_usage("--subject-prefix [Subject-Prefix] 'Use [Subject-Prefix] instead of the standard [PATCH] prefix'"),

If I use either the --subject-prefix or -v options alone, without specifying a value, it complains:

ajd@ajd:~/code/skiboot$ /home/ajd/code/git-series/target/debug/git-series format --subject-prefix 
error: The argument '--subject-prefix <Subject-Prefix>' requires a value but none was supplied
...
ajd@ajd:~/code/skiboot$ /home/ajd/code/git-series/target/debug/git-series format -v
error: The argument '--reroll-count <N>' requires a value but none was supplied
...

But if I use --subject-prefix followed by -v 2, it gives me an empty value for --subject-prefix:

ajd@ajd:~/code/skiboot$ /home/ajd/code/git-series/target/debug/git-series format --subject-prefix -v 2
v2-0000-cover-letter.patch
v2-0001-hw-phb3-add-OPAL-call-to-get-current-PHB-CAPI-state.patch
...

And vice versa:

ajd@ajd:~/code/skiboot$ /home/ajd/code/git-series/target/debug/git-series format -v --subject-prefix banana
0000-cover-letter.patch
0001-hw-phb3-add-OPAL-call-to-get-current-PHB-CAPI-state.patch
...

This is shown in both 2.10.0 and 2.12.1.

(Apologies for the poor nature of this report, I'm in the middle of yak shaving so I may well have missed something here.)

[kbknapp]

Thanks for filing this report, this looks like a bug and I'll start investigating it! There may be a way to force what you're looking for by using the Arg::from_usage instead of App::arg_from_usage, because that allows you setting additional properties like, Arg::number_of_values or other such settings.

Also, a side note not related to this issue, if you have multiple from_usage args to declare which don't require any of the additional Arg methods you can use App::args_from_usage to save some typing:

.args_from_usage("
        --in-reply-to [Message-Id] 'Make the first mail a reply to the specified Message-Id'
        --no-from 'Don't include in-body \"From:\" headers when formatting patches authored by others'
    -v, --reroll-count=[N] 'Mark the patch series as PATCH vN'
        --stdout 'Write patches to stdout rather than files'
        --subject-prefix [Subject-Prefix] 'Use [Subject-Prefix] instead of the standard [PATCH] prefix'")

[kbknapp]

@ajdlinux I'm not able to reproduce this. Can you post a link to the file where this happening, it may be related to another setting or something that you're using.

This is what I used to test

[ajdlinux]

Branch: https://github.com/ajdlinux/git-series/tree/format-subject-prefix
File: https://github.com/ajdlinux/git-series/blob/format-subject-prefix/src/main.rs

I'm not very familiar with this codebase (forked it just to add a single new option) so there might be something in there I'm missing. Will look into it shortly when I've got a bit more time.

[kbknapp]

@ajdlinux sorry it's take so long! I've been doing a lot of traveling and then the holidays came around. I'm finally able to reproduce this, and it's next up on the fix list! I'll let you know what I find.

[kbknapp]

This is fixed in #796

One thing to note is that you should add Arg::empty_values(false) to --subject-prefix because by default clap allows empty values unless you specificy a minimum number of values.

[ajdlinux]

Thanks @kbknapp, apologies for not following this up, I'd forgotten about this!