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

multiple args not working with clap derive on Vec<Foo> type #3283

Closed
2 tasks done
tdudz opened this issue Jan 11, 2022 · 1 comment
Closed
2 tasks done

multiple args not working with clap derive on Vec<Foo> type #3283

tdudz opened this issue Jan 11, 2022 · 1 comment
Labels
A-derive Area: #[derive]` macro API C-bug Category: Updating dependencies S-duplicate Status: Closed as Duplicate

Comments

@tdudz
Copy link

tdudz commented Jan 11, 2022

Please complete the following tasks

  • I have searched the discussions
  • I have searched the existing issues

Rust Version

1.57.0

Clap Version

3.0.6

Minimal reproducible code

#[derive(Parser)]
struct Args {
    #[clap(long, short)]
    foo: Foo,
    #[clap(long, short)]
    bars: Vec<Bar>,
}

fn main() {
    let args = Args::parse();
}

Steps to reproduce the bug with the above code

cargo run -f foo1 -b bar1 bar2

Actual Behaviour

error: Found argument 'bar2' which wasn't expected, or isn't valid in this context

Expected Behaviour

it should parse bar1 and bar2 and place them into a Vec<Bar>

Additional Context

No response

Debug Output

No response

@tdudz tdudz added the C-bug Category: Updating dependencies label Jan 11, 2022
@epage
Copy link
Member

epage commented Jan 11, 2022

In clap 3, we changed Vec from multiple to multiple_occurrences. This is called out in the CHANGELOG's breaking changes since structopt. This was planned in #1772.

For reference, we also have #3248 for this.

@epage epage closed this as completed Jan 11, 2022
@epage epage added A-derive Area: #[derive]` macro API S-duplicate Status: Closed as Duplicate labels Jan 11, 2022
dfinity-bot pushed a commit to dfinity/ic that referenced this issue Apr 7, 2022
[rc] Bump clap version v2

The last MR caused issues in the release pipeline, specifically with `ic-admin`. The CLI bump caused an unexpected behavior change. I located the issue and decided to try again 😄 

The breaking change that caused issues is the following:\
The new version changed the default for passing multiple values and Clap now complains if multiple values are passed to an argument. It affects CLI options that are `Vec<_>`. In the CI it was the `--replacement-nodes` that is a `Vec<_>`.

In the previous version you could do:
`ic-admin --values v1 v2 v3`  -> values: `Some("v1","v2","v3")` \
In the current version (v3) they changed the default behavior to:\
`ic-admin --values v1 --values v2 --values v3` or `#derive[clap(multiple_values(true))]`

To keep the old behaviour I added `#derive[clap(multiple_values(true))]` to the arguments that use a `Vec<_>`. It should now behave the same way as with the older `clap` version.

Ref:
- [Similar issue](clap-rs/clap#3283)
- [Similar issue](clap-rs/clap#3248) 
- [Clap changelog](https://github.com/clap-rs/clap/blob/master/CHANGELOG.md#breaking-changes)

[Issue](https://dfinity.slack.com/archives/C024WJPPX5W/p1648646423132219?thread_ts=1648605012.761379&cid=C024WJPPX5W): 
```
Start: Wed Mar 30 13:08:14 UTC 2022
error: Found argument '4evoo-kgi22-4rxri-rtibv-ny4u4-qy6bw-7ivmv-vw3iw-ij3jy-dxrof-tae' which wasn't expected, or isn't valid in this context
USAGE:
    ic-admin --nns-url <NNS_URL> propose-to-update-recovery-cup [OPTIONS] --subnet <SUBNET> --height <HEIGHT> --time-ns <TIME_NS> --state-hash <STATE_HASH>
For more information try --help
```
Next steps:
-  remove `structopt` since it now merged with clap v3 

See merge request dfinity-lab/public/ic!4095
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-derive Area: #[derive]` macro API C-bug Category: Updating dependencies S-duplicate Status: Closed as Duplicate
Projects
None yet
Development

No branches or pull requests

2 participants