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

Flag also parsed as positional argument #946

Closed
Mark-Simulacrum opened this issue May 5, 2017 · 4 comments
Closed

Flag also parsed as positional argument #946

Mark-Simulacrum opened this issue May 5, 2017 · 4 comments
Labels
C-bug Category: Updating dependencies
Milestone

Comments

@Mark-Simulacrum
Copy link

Mark-Simulacrum commented May 5, 2017

Rust Version

N/A, but tested on latest nightly

Affected Version of clap

2.22.1

Expected Behavior Summary

With a single, optional positional argument and a flag, we don't parse the flag as the positional argument. I also set AllowLeadingHyphen, since some of my arguments start with -C.

Actual Behavior Summary

The optional positional argument is parsed as Some("--verbose") instead of None, and --verbose is also parsed.

Steps to Reproduce the issue

I'm not really sure what to put here, but basically

app --verbose is parsed as:

  • is_present("--verbose") = true
  • `value_of("filter") == Some("--verbose")

Sample Code or Link to Sample Code

https://github.com/Mark-Simulacrum/rust/tree/cleanup is the branch, but you'll need to cd to src/tools/compiletest and then just cargo build should work.

After that, running this command reproduces the issue, pay attention to how filter is printed as Some("--verbose") in the debug output:

./src/target/debug/compiletest "--compile-lib-path" "./rust/build/x86_64-unknown-linux-gnu/stage1/lib" "--run-lib-path" "./rust/build/x86_64-unknown-linux-gnu/stage1/lib/rustlib/x86_64-unknown-linux-gnu/lib" "--rustc-path" "./rust/build/x86_64-unknown-linux-gnu/stage1/bin/rustc" "--rustdoc-path" "./rust/build/x86_64-unknown-linux-gnu/stage1/bin/rustdoc" "--src-base" "./rust/src/test/compile-fail" "--build-base" "./rust/build/x86_64-unknown-linux-gnu/test/compile-fail" "--stage-id" "stage1-x86_64-unknown-linux-gnu" "--mode" "compile-fail" "--target" "x86_64-unknown-linux-gnu" "--host" "x86_64-unknown-linux-gnu" "--llvm-filecheck" "./rust/build/x86_64-unknown-linux-gnu/llvm/build/bin/FileCheck" "--nodejs" "/usr/bin/node" "--host-rustcflags" "-Crpath -O" "--target-rustcflags" "-Crpath -O -Lnative=./rust/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--docck-python" "/usr/bin/python2.7" "--lldb-python" "/usr/bin/python2.7" "--gdb" "/usr/bin/gdb" "--llvm-version" "4.0.1\n" "--verbose" "--cc" "" "--cxx" "" "--cflags" "" "--llvm-components" "" "--llvm-cxxflags" "" "--adb-path" "adb" "--adb-test-dir" "/data/tmp/work" "--android-cross-path" ""

Debug output

https://gist.github.com/Mark-Simulacrum/b86c4f16f922be7ef542e7853cf93e01, with what I think being relevant here: https://gist.github.com/Mark-Simulacrum/b86c4f16f922be7ef542e7853cf93e01#file-gistfile1-txt-L1590

@kbknapp
Copy link
Member

kbknapp commented May 6, 2017

Thanks for taking the time to file this! I've confirmed the bug, and it should be an easy one to fix.

Also, just to note AllowLeadingHyphen is for when values may start with a hyphen, and should be used sparingly if possible. A better option is to use allow_hyphen_values only on the args who's values may start with a leading hyphen. It's not critical, I just wanted to note it (and if you already knew this I apologize 😄)

@kbknapp kbknapp added D: easy C-bug Category: Updating dependencies labels May 6, 2017
@kbknapp kbknapp added this to the 2.24.1 milestone May 6, 2017
@Mark-Simulacrum
Copy link
Author

Great! Thanks for pointing out that there's a difference between AllowLeadingHypen and allow_hyphen_values, but I think I'm confused (feel free to tell me I should move this to another issue). The documentation for AllowLeadingHyphen suggests that I have to set that globally: "NOTE: This can only be set application wide and not on a per argument basis." But what you're saying seems to suggest that's not the case?

@kbknapp
Copy link
Member

kbknapp commented May 6, 2017

there's a difference between AllowLeadingHypen and allow_hyphen_values, but I think I'm confused

They do the same thing, one is globally (AllowLeadingHyphen) and the other is per argument (allow_hyphen_values).

The documentation is out of date (embarrassed). That was true before allow_hyphen_values was introduced. I'm updating it as we speak.

kbknapp added a commit that referenced this issue May 6, 2017
…es when specific combinations of settings were used

This commit fixes a bug where using `AppSettings::AllowHyphenValues`
would cause flags with longs be *also* parsed as positional values.

Closes #946
@kbknapp kbknapp closed this as completed in fedb46b May 7, 2017
@kbknapp
Copy link
Member

kbknapp commented May 7, 2017

2.24.1 is on crates.io

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: Updating dependencies
Projects
None yet
Development

No branches or pull requests

2 participants