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

Prevent multiple occurrences of --path from eating up the subcommand #224

Merged
merged 1 commit into from
Apr 4, 2024

Conversation

naiquevin
Copy link
Contributor

@naiquevin naiquevin commented Apr 2, 2024

In case the --path option was specified multiple times, it'd consider the subcommand as yet another path value, thereby causing the subcommand to default to Check.

Steps to reproduce:

  $ mkdir /tmp/{foo,bar}
  $ echo "[tag: hello]" > /tmp/foo/file.txt
  $ echo "[ref: hello]" > /tmp/bar/file.txt
  $ tagref -p /tmp/foo -p /tmp/bar list-tags
  1 tag, 1 tag reference, 0 file references, and 0 directory references validated in 2 files.

Notice that it prints the output of check subcommand instead of list-tags. This happens because the default behavior Args::multiple(true) in clap (version 2.x) is to allow multiple values per occurrence of the option.

Setting Args::number_of_values(1) in coordination with Args::multiple(true) fixes it. It allows multiple occurrences of the option but only one value per occurrence1.

In case the '--path' option was specified multiple times, it'd
consider the subcommand as yet another path value, thereby causing the
subcommand to default to 'Check'.

Steps to reproduce:

  $ mkdir /tmp/{foo,bar}
  $ echo "[tag: hello]" > /tmp/foo/file.txt
  $ echo "[ref: hello]" > /tmp/bar/file.txt
  $ tagref -p /tmp/foo -p /tmp/bar list-tags
  1 tag, 1 tag reference, 0 file references, and 0 directory references validated in 2 files.

Notice that it prints the output of check subcommand instead of
list-tags. This happens because the default behavior
'Args::multiple(true)' in clap (version 2.x) is to allow multiple
values per occurrence of the option.

Setting 'Args::number_of_values(1)' in coordination with
'Args::multiple(true)' fixes it. It allows multiple occurrences of the
option but only one value per occurrence[1].

[1]: https://docs.rs/clap/2.34.0/clap/struct.Arg.html#method.multiple
@stepchowfun
Copy link
Owner

Thank you @naiquevin for fixing this issue! I will take a closer look later, but right now it looks good to me.

@stepchowfun stepchowfun merged commit 92fbf4a into stepchowfun:main Apr 4, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants