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

`run` doesn't pass empty arguments #190

Closed
brson opened this Issue Mar 29, 2016 · 11 comments

Comments

Projects
None yet
2 participants
@brson
Copy link
Contributor

brson commented Mar 29, 2016

Source

rustup run stable cargo build --verbose --no-default-features --features ""
Expected argument for flag '--features' but reached end of arguments.
@brson

This comment has been minimized.

Copy link
Contributor Author

brson commented Mar 29, 2016

This is probably a bug in clap or me not understanding clap. The code is pretty straightforward. values_of("command") is supposed to return multiple arguments but doesn't return the trailing empty argument. cc @kbknapp does this look familiar to you?

@brson

This comment has been minimized.

Copy link
Contributor Author

brson commented Mar 29, 2016

"command" is defined as Arg::with_name("command").required(true).multiple(true))

@kbknapp

This comment has been minimized.

Copy link
Contributor

kbknapp commented Mar 30, 2016

@brson I think that's a docopt error message, or at least it's not one from clap. The error clap would have given in the same scenario (expecting an argument value but not getting one) would be:

$ fake-rs --features
error: The argument '--features <feature>...' requires a value but none was supplied

USAGE:
    fake-rs [FLAGS] --features <feature>...

For more information try --help

I also just tested it with the empty string: --features "" which doesn't give an error. Empty values are allowed by default.

Side note, you can force it to be an error with the Arg::empty_values(false) if that's desired.

@kbknapp

This comment has been minimized.

Copy link
Contributor

kbknapp commented Mar 30, 2016

@brson Thinking about it, I think it's because those arguments are being passed to cargo, which explains the error message.

@brson

This comment has been minimized.

Copy link
Contributor Author

brson commented Mar 30, 2016

Yes it's because the error came from cargo. I'll keep looking into it. Thanks

@kbknapp

This comment has been minimized.

Copy link
Contributor

kbknapp commented Mar 30, 2016

@brson Ah ok, I misunderstood how it was running, I wasn't thinking the entire stable cargo build --verbose --no-default-features --features "" was being passed. This may in fact be a bug, let me look into it a little deeper I'll get back.

@brson

This comment has been minimized.

Copy link
Contributor Author

brson commented Mar 30, 2016

@kbknapp

This comment has been minimized.

Copy link
Contributor

kbknapp commented Mar 30, 2016

Thanks! Checking it out now.

@kbknapp kbknapp referenced this issue Mar 30, 2016

Merged

Issue 470 #471

@kbknapp

This comment has been minimized.

Copy link
Contributor

kbknapp commented Mar 30, 2016

Just put in a PR to fix this!

Edit: I'll post back here once the new version is on crates.io (should be ~30 mins)

homu added a commit to clap-rs/clap that referenced this issue Mar 30, 2016

Auto merge of #471 - kbknapp:issue-470, r=kbknapp
Issue 470

Relates to [multirust-rs 190](rust-lang/rustup.rs#190)

homu added a commit to clap-rs/clap that referenced this issue Mar 30, 2016

Auto merge of #471 - kbknapp:issue-470, r=kbknapp
Issue 470

Relates to [multirust-rs 190](rust-lang/rustup.rs#190)
@kbknapp

This comment has been minimized.

Copy link
Contributor

kbknapp commented Mar 30, 2016

v2.2.4 is on crates.io

@brson

This comment has been minimized.

Copy link
Contributor Author

brson commented Mar 31, 2016

\o/ @kbknapp Thanks for the quick update! I'm pulling it in now.

brson added a commit to brson/rustup.rs that referenced this issue Mar 31, 2016

bors added a commit that referenced this issue Mar 31, 2016

Auto merge of #229 - brson:empty-args, r=alexcrichton
Upgrade clap and add test that `run` accepts empty args. Fixes #190

alexcrichton added a commit that referenced this issue Apr 1, 2016

Merge pull request #229 from brson/empty-args
Upgrade clap and add test that `run` accepts empty args. Fixes #190
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.