Mirrors/clap
2
0
Fork 0
mirror of https://github.com/clap-rs/clap synced 2024-12-13 22:32:33 +00:00
Code Issues Projects Releases Packages Wiki Activity

fix: when an argument requires a value and that value happens to match a subcommand name, its parsed as a value

Browse source 
**This commit contains a breaking change in order to fix a bug.**

The only users affected are those relying on the "bug." The bug is only in code that uses
subcommands, and parent commands with arguments accepting multiple values (positionals or options)
unrestrained *and* where a value may coincide with a subcommand's name.

Imagine a CLI with a positional argument `files` that accepts multiple values but no other conditions
or parameters (just `Arg::multiple(true)`), and a subcommand `log`

Prior to this commit, the following was valid:

```
$ prog file1 file2 file3 log
```

which would be parsed as:

 * files = file1, file2, file3
 * subcommand = log

However, if there was a file named `log` the subcommand isn't callable.

The CLI should be designed in a way that either limits number of values so that clap knows
when `files` is done and can then look for subcommands, or other AppSettings which change
the behavior.

Possible fixes are `Arg::number_of_values`, `Arg::max_values`,
`AppSettings::ArgsNegateSubcommands`, `Arg::require_equals`,
`AppSettings::SubcommandsNegateArgs`, and more.

This *does not* affect CLIs which contain other arguments between the unrestrained multiple
value args, and the any subcommands. Such as if our example above also had a flag `-f`

The following would be parsed the same before and after this commit.

```
$ prog file1 file2 file3 -f log
```

This is because upon reaching the flag `-f`, clap stops parsing `files`.

No other breaking changes were made.

---

When using args with `multiple(true)` on options or positionals (i.e. those args that
accept values) and subcommands, one needs to consider the posibility of an argument value
being the same as a valid subcommand. By default `clap` will parse the argument in question
as a value *only if* a value is possible at that moment. Otherwise it will be parsed as a
subcommand. In effect, this means using `multiple(true)` with no additional parameters and
a possible value that coincides with a subcommand name, the subcommand cannot be called
unless another argument is passed first.

As an example, consider a CLI with an option `--ui-paths=<paths>...` and subcommand `signer`

The following would be parsed as values to `--ui-paths`.

```
$ program --ui-paths path1 path2 signer
```

This is because `--ui-paths` accepts multiple values. `clap` will continue parsing values
until another argument is reached and it knows `--ui-paths` is done.

By adding additional parameters to `--ui-paths` we can solve this issue. Consider adding
`Arg::number_of_values(1)` as discussed above. The following are all valid, and `signer`
is parsed as both a subcommand and a value in the second case.

```
$ program --ui-paths path1 signer
$ program --ui-paths path1 --ui-paths signer signer
```

Closes #1031
This commit is contained in:
Kevin K 2017-10-24 13:57:20 -07:00
parent b399ee2604
commit 0c223f54ed
No known key found for this signature in database
GPG key ID: 17218E4B3692F01A
1 changed files with 15 additions and 10 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

25
src/app/parser.rs
View file

@ -848,17 +848,22 @@ impl<'a, 'b> Parser<'a, 'b>
if !self.is_set(AS::TrailingValues) {
// Does the arg match a subcommand name, or any of it's aliases (if defined)
{
let (is_match, sc_name) = self.possible_subcommand(&arg_os);
debugln!("Parser::get_matches_with: possible_sc={:?}, sc={:?}",
is_match,
sc_name);
if is_match {
let sc_name = sc_name.expect(INTERNAL_ERROR_MSG);
if sc_name == "help" && self.is_set(AS::NeedsSubcommandHelp) {
self.parse_help_subcommand(it)?;
match needs_val_of {
ParseResult::Opt(_) | ParseResult::Pos(_) =>(),
_ => {
let (is_match, sc_name) = self.possible_subcommand(&arg_os);
debugln!("Parser::get_matches_with: possible_sc={:?}, sc={:?}",
is_match,
sc_name);
if is_match {
let sc_name = sc_name.expect(INTERNAL_ERROR_MSG);
if sc_name == "help" && self.is_set(AS::NeedsSubcommandHelp) {
self.parse_help_subcommand(it)?;
}
subcmd_name = Some(sc_name.to_owned());
break;
}
}
subcmd_name = Some(sc_name.to_owned());
break;
}
}