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

A .required(true).takes_value(true) arg is successfully assigned no value if followed by a recognized option #1105

Closed
ExpHP opened this issue Nov 13, 2017 · 5 comments
Labels
C-bug Category: Updating dependencies

Comments

@ExpHP
Copy link

ExpHP commented Nov 13, 2017

Rust Version

  • rustc 1.23.0-nightly (ee2286149 2017-11-07)

Affected Version of clap

  • 2.27.1

Expected Behavior Summary

If an argument has the following settings:

required: true
takes_value: true
multiple: false

then value_of("argument") should never be None. (get_matches should fail before that ever happens)

Actual Behavior Summary

If such an argument is followed by a valid option, then get_matches() succeeds, but value_of("argument") is None.

(note: arguments with allow_hyphen_values(true) are not affected, as in that case it interprets the option as being the argument's value)

Sample Code or Link to Sample Code

playground

extern crate clap;

fn main() {
    use ::clap::{App, Arg};
    let matches = App::new("prog")
        .arg(Arg::with_name("pat")
            .takes_value(true)
            .required(true)
            .multiple(false)
            .allow_hyphen_values(false)
            .long("pattern"))
        .arg(Arg::with_name("force")
            .short("f"))
        .get_matches_from(vec![
            "prog", "--pattern", "-f"
        ]);
    
    assert!(matches.value_of("pat").is_some());
}
thread 'main' panicked at 'assertion failed: matches.value_of("pat").is_some()', src/main.rs:17:4

Debug output

DEBUG:clap:Parser::propagate_settings: self=prog, g_settings=AppFlags(                                                                                       
    NEEDS_LONG_HELP | NEEDS_LONG_VERSION | NEEDS_SC_HELP | UTF8_NONE | COLOR_AUTO                                                                            
)                                                                                                                                                            
DEBUG:clap:Parser::get_matches_with;
DEBUG:clap:Parser::create_help_and_version;
DEBUG:clap:Parser::create_help_and_version: Building --help
DEBUG:clap:Parser::create_help_and_version: Building --version
DEBUG:clap:Parser::get_matches_with: Begin parsing '"--pattern"' ([45, 45, 112, 97, 116, 116, 101, 114, 110])
DEBUG:clap:Parser::is_new_arg: arg="--pattern", Needs Val of=NotFound
DEBUG:clap:Parser::is_new_arg: Arg::allow_leading_hyphen(false)
DEBUG:clap:Parser::is_new_arg: -- found
DEBUG:clap:Parser::is_new_arg: starts_new_arg=true
DEBUG:clap:Parser::possible_subcommand: arg="--pattern"
DEBUG:clap:Parser::get_matches_with: possible_sc=false, sc=None
DEBUG:clap:Parser::parse_long_arg;
DEBUG:clap:Parser::parse_long_arg: Does it contain '='...No
DEBUG:clap:OptBuilder::fmt:pat
DEBUG:clap:Parser::parse_long_arg: Found valid opt '--pattern <pat>'
DEBUG:clap:Parser::parse_opt; opt=pat, val=None
DEBUG:clap:Parser::parse_opt; opt.settings=ArgFlags(REQUIRED | EMPTY_VALS | TAKES_VAL | DELIM_NOT_SET)
DEBUG:clap:Parser::parse_opt; Checking for val...None
DEBUG:clap:ArgMatcher::inc_occurrence_of: arg=pat
DEBUG:clap:ArgMatcher::inc_occurrence_of: first instance
DEBUG:clap:Parser::groups_for_arg: name=pat
DEBUG:clap:Parser::groups_for_arg: No groups defined
DEBUG:clap:Parser::parse_opt: More arg vals required...
DEBUG:clap:arg_post_processing!;
DEBUG:clap:OptBuilder::fmt:pat
DEBUG:clap:arg_post_processing!: Is '--pattern <pat>' in overrides...No
DEBUG:clap:OptBuilder::fmt:pat
DEBUG:clap:arg_post_processing!: Does '--pattern <pat>' have overrides...No
DEBUG:clap:OptBuilder::fmt:pat
DEBUG:clap:arg_post_processing!: Does '--pattern <pat>' have conflicts...No
DEBUG:clap:OptBuilder::fmt:pat
DEBUG:clap:arg_post_processing!: Does '--pattern <pat>' have requirements...No
DEBUG:clap:_handle_group_reqs!;
DEBUG:clap:Parser:get_matches_with: After parse_long_arg Opt("pat")
DEBUG:clap:Parser::get_matches_with: Begin parsing '"-f"' ([45, 102])
DEBUG:clap:Parser::is_new_arg: arg="-f", Needs Val of=Opt("pat")
DEBUG:clap:Parser::is_new_arg: Arg::allow_leading_hyphen(false)
DEBUG:clap:Parser::is_new_arg: - found
DEBUG:clap:Parser::is_new_arg: starts_new_arg=true
DEBUG:clap:Parser::parse_short_arg: full_arg="-f"
DEBUG:clap:Parser::parse_short_arg:iter:f
DEBUG:clap:Parser::parse_short_arg:iter:f: Found valid flag
DEBUG:clap:Parser::check_for_help_and_version_char;
DEBUG:clap:Parser::check_for_help_and_version_char: Checking if -f is help or version...Neither
DEBUG:clap:Parser::parse_flag;
DEBUG:clap:ArgMatcher::inc_occurrence_of: arg=force
DEBUG:clap:ArgMatcher::inc_occurrence_of: first instance
DEBUG:clap:Parser::groups_for_arg: name=force
DEBUG:clap:Parser::groups_for_arg: No groups defined
DEBUG:clap:arg_post_processing!;
DEBUG:clap:arg_post_processing!: Is '-f' in overrides...No
DEBUG:clap:arg_post_processing!: Does '-f' have overrides...No
DEBUG:clap:arg_post_processing!: Does '-f' have conflicts...No
DEBUG:clap:arg_post_processing!: Does '-f' have requirements...No
DEBUG:clap:_handle_group_reqs!;
DEBUG:clap:Parser:get_matches_with: After parse_short_arg Flag
DEBUG:clap:Validator::validate;
DEBUG:clap:Parser::add_defaults;
DEBUG:clap:Parser::add_defaults:iter:pat: doesn't have conditional defaults
DEBUG:clap:Parser::add_defaults:iter:pat: doesn't have default vals
DEBUG:clap:Validator::validate_blacklist: blacklist=[]
DEBUG:clap:Validator::validate_required: required=["pat"];
DEBUG:clap:Validator::validate_required:iter:pat:
DEBUG:clap:Validator::validate_matched_args;
DEBUG:clap:Validator::validate_matched_args:iter:pat: vals=[]
DEBUG:clap:Validator::validate_arg_num_vals;
DEBUG:clap:Validator::validate_values: arg="pat"
DEBUG:clap:Validator::validate_arg_requires;
DEBUG:clap:Validator::validate_arg_num_occurs: a=pat;
DEBUG:clap:Validator::validate_matched_args:iter:force: vals=[]
DEBUG:clap:Validator::validate_arg_requires;
DEBUG:clap:Validator::validate_arg_num_occurs: a=force;
DEBUG:clap:usage::create_usage_with_title;
DEBUG:clap:usage::create_usage_no_title;
DEBUG:clap:usage::get_required_usage_from: reqs=["pat"], extra=None
DEBUG:clap:usage::get_required_usage_from: after init desc_reqs=[]
DEBUG:clap:usage::get_required_usage_from: no more children
DEBUG:clap:usage::get_required_usage_from: final desc_reqs=["pat"]
DEBUG:clap:usage::get_required_usage_from: args_in_groups=[]
DEBUG:clap:usage::get_required_usage_from:iter:pat:
DEBUG:clap:OptBuilder::fmt:pat
DEBUG:clap:usage::needs_flags_tag;
DEBUG:clap:usage::needs_flags_tag:iter: f=force;
DEBUG:clap:Parser::groups_for_arg: name=force
DEBUG:clap:Parser::groups_for_arg: No groups defined
DEBUG:clap:usage::needs_flags_tag:iter: [FLAGS] required
DEBUG:clap:usage::create_help_usage: usage=prog [FLAGS] --pattern <pat>
DEBUG:clap:ArgMatcher::get_global_values: global_arg_vec=[]
@ExpHP
Copy link
Author

ExpHP commented Nov 13, 2017

Note: This looks suspiciously like #665 which was supposedly fixed.

Adding .empty_values(false) as mentioned by @kbknapp near the end causes it to (correctly) fail with

error: The argument '--pattern <pat>' requires a value but none was supplied

but I don't see how there is an "empty value" here, nor can I possibly imagine why an empty value would produce None instead of Some("").

// by my measure...
["prog", "--pattern=", "-f"]    // has an empty value
["prog", "--pattern", "", "-f"] // has an empty value
["prog", "--pattern", "-f"]     // no empty value here (args in sample code above)

@kbknapp
Copy link
Member

kbknapp commented Nov 13, 2017

Thanks for reporting this and all the details!

You've correctly assessed everything. The problem stems from clap conflating two subjectively distinct versions of "empty values."

As time has gone on, I've grown more opinionated about what an "empty value" is, and what should be allowed by default. I'll spare the history of why it is the way it is currently, and just state what I think it should be going forward to fix this.

An empty value is one that is explicitly and intentionally blank. This would mean only the following would qualify as an empty value:

  • --option ""
  • --option=""
  • --option=
  • -o=""
  • -o=
  • -o ""
  • -o""

Note: --option="" and --option= are actually the same thing as the shell strips "" from the end. Same with -o= and -o="". This also means -o"" and -o are actually the same as well.

Note 2: depending on the shell a single quote may work as well (')`

Currently, the following also qualify as an empty value, which I'm finally declaring as a bug:

  • --option
  • -o

Here's the summary in table form (assuming empty_values(true), which is default):

Invocation Current Future
--option None Bug
--option "" Some("")
--option= / --option="" Some("")
-o None Bug
-o "" Some("")
-o"" None Bug
-o= / -o="" Some("=") Bug

Note the strange findings with -o= and o="" which is absolutely a bug, but of a different nature.

@kbknapp kbknapp added C: options C-bug Category: Updating dependencies labels Nov 13, 2017
@ExpHP
Copy link
Author

ExpHP commented Nov 13, 2017

In my opinion, it feels funny to even be considering "" as having its own meaning in arguments, since my mind just naturally runs through the shell expansion rules; when you write -o"", I see -o. 😜 (or perhaps, "-o")

I'm glad that you plan to change this case to an error, because it feels to me like a recipe for trouble to be trying to guess the user's intent based on guessing where quotes used to be in the input.

@kbknapp
Copy link
Member

kbknapp commented Nov 13, 2017

In my opinion, it feels funny to even be considering "" as having its own meaning in arguments, since my mind just naturally runs through the shell expansion rules; when you write -o"", I see -o

The way Rust (or shells in general?) pass the argv to clap, --option "" sends two distinct values, --option and "". Whereas --option="" the shell strips the "" as you stated.

Some users want the ability to pass in empty values (i.e. when a value should be required, however sometimes a blank value is what is desired).

@kbknapp
Copy link
Member

kbknapp commented Nov 13, 2017

This is fixed in #1109

@kbknapp kbknapp mentioned this issue Nov 14, 2017
87 tasks
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