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

fix(parser): AllowHyphenValues correctly handles long args in first pos #4039

Closed

Conversation

emersonford
Copy link
Contributor

@emersonford emersonford commented Aug 8, 2022

pretty much just @epage's #3389, but with a bunch of unit tests

fixes #3880
fixes #1538
fixes #3887 (specifically the "... none of them seem to allow for the first argument to have two hyphens ..." part)

rephrasing this fix to specifically focus on allow_hyphen_values(true) as it's the root cause of the bug in #1538

This specifically only changes the parsing behavior when allow_hyphen_values(true) is used. Iow, the parsing behavior of setting say trailing_var_args(true) without setting allow_hyphen_values(true) does not change.

A note on whether this is a "bug fix", the documentation for Arg::allow_hyphen_values assumes that prog --arg -- -- val works (it doesn't without this PR), so I personally feel this can be reasonably treated as a bug fix and not an explicit change in behavior to users?

Behaviors that change with this PR

  1. Using allow_hyphen_values(true) on a Command that has optional long flags: mistyped/unrecognized optional long flags will always be treated as argument values and will not surface unrecognized flag errors
  2. Using trailing_var_args(true) on a Command that has allow_hyphen_values(true) set on the final Arg: mistyped/unrecognized long flag will result in that flag, plus all subsequent args, being treated as values

@epage
Copy link
Member

epage commented Aug 9, 2022

FYI from my testing, adding Command::allow_hyphen_values(true) fixes this behavior, see #1538 (comment)

In my mind, this de-prioritizes it a little bit so we can focus on big questions like

@ModProg
Copy link
Contributor

ModProg commented Sep 1, 2022

Interestingly enough, using it on app level only seams to fix the problem, if the long flag does not exist in the command.

i.e. a random --smth will be a value, but --help triggers the help message.

Maybe there is a workaround for my problem as well, I have a program that takes a script name and args for the script:

#[clap(allow_hyphen_values = true)]
struct Option{
  /// The script 
  script: PathBuf,
  #[clap(allow_hyphen_values = true)]
  args: Vec<OsString>
}

when I put e.g. --version behind the script it ends up in args, but --help does not and prints the help instead.

@emersonford
Copy link
Contributor Author

@ModProg you need to disable the help flag with disable_help_flag and --help will be treated as a value

@ModProg
Copy link
Contributor

ModProg commented Sep 2, 2022

@ModProg you need to disable the help flag with disable_help_flag and --help will be treated as a value

I know, but I want ro have --help as well

epage pushed a commit to epage/clap that referenced this pull request Sep 7, 2022
…first pos

This makes it match up with `Command::allow_hyphen_values` which was the
guiding factor for what the behavior should be.

This supersedes clap-rs#4039

Fixes clap-rs#3880
Fixes clap-rs#1538
@epage
Copy link
Member

epage commented Sep 7, 2022

I'm feeling fairly confident in the behavior in #4187

@ModProg
Copy link
Contributor

ModProg commented Sep 8, 2022

I'm feeling fairly confident in the behavior in #4187

works for me

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants