clap icon indicating copy to clipboard operation
clap copied to clipboard

Optional option-argument parsing is incorrect

Open mkayaalp opened this issue 2 years ago • 20 comments

Please complete the following tasks

  • [X] I have searched the discussions
  • [X] I have searched the existing issues

Rust Version

rustc 1.56.0 (09c42c458 2021-10-18)

Clap Version

master

Minimal reproducible code

use clap::Parser;
#[derive(Parser, Debug)]
struct Opt {
    #[clap(short,long)]
    foo: Option<Option<String>>,
    input: String,
}

fn main() {
    let opt = Opt::parse();
    println!("{:?}", opt);
}

Steps to reproduce the bug with the above code

cargo run -- -f input

or

cargo run -- --foo input

Actual Behaviour

error: The following required arguments were not provided:
    <INPUT>

USAGE:
    clap-test --foo <FOO> <INPUT>

For more information try --help

Expected Behaviour

Opt { foo: Some(None), input: "input" }

Additional Context

There needs to be some special handling of options with optional arguments.

Optional option-arguments are bad

First of all, users really need to be discouraged (if we ever add some lint-like feature, e.g. #[clap(enable-warnings)).

POSIX says:

Option-arguments should not be optional.

Solaris says:

Option-arguments cannot be optional.

Parsing

I know some people have a preference for -f OptArg over -fOptArg, but if the option-argument is optional, then only the latter is correct. If there is a space, it means the optional argument is not there.

From POSIX.1-2017 - Ch. 12: Utility Conventions:

a conforming application shall place any option-argument for that option directly adjacent to the option in the same argument string, without intervening <blank> characters

If the utility receives an argument containing only the option, it shall behave as specified in its description for an omitted option-argument; it shall not treat the next argument (if any) as the option-argument for that option.

Same thing with the long option, which is a GNU extension.

From GNU C Library Reference Manual - Ch. 25.1.1: Program Argument Syntax Conventions:

To specify an argument for a long option, write --name=value. This syntax enables a long option to accept an argument that is itself optional.

See this SO question about how getopt handles this: getopt does not parse optional arguments to parameters.

In clap terms, these are saying we should only allow

  • --long
  • --long=value
  • -s
  • -svalue

In contrast, clap supports

  • Default:
    • --long
    • --long=value
    • --long value
    • -s
    • -s value
    • -s=value
    • -svalue
  • requires_equals(true)
    • --long
    • --long=value
    • -s
    • -s=value

Debug Output

[            clap::build::app] 	App::_do_parse
[            clap::build::app] 	App::_build
[            clap::build::app] 	App::_propagate:clap-test
[            clap::build::app] 	App::_check_help_and_version
[            clap::build::app] 	App::_check_help_and_version: Removing generated version
[            clap::build::app] 	App::_propagate_global_args:clap-test
[            clap::build::app] 	App::_derive_display_order:clap-test
[clap::build::app::debug_asserts] 	App::_debug_asserts
[clap::build::arg::debug_asserts] 	Arg::_debug_asserts:help
[clap::build::arg::debug_asserts] 	Arg::_debug_asserts:foo
[clap::build::arg::debug_asserts] 	Arg::_debug_asserts:input
[         clap::parse::parser] 	Parser::get_matches_with
[         clap::parse::parser] 	Parser::_build
[         clap::parse::parser] 	Parser::_verify_positionals
[         clap::parse::parser] 	Parser::get_matches_with: Begin parsing 'RawOsStr("-f")' ([45, 102])
[         clap::parse::parser] 	Parser::get_matches_with: Positional counter...1
[         clap::parse::parser] 	Parser::get_matches_with: Low index multiples...false
[         clap::parse::parser] 	Parser::possible_subcommand: arg=RawOsStr("-f")
[         clap::parse::parser] 	Parser::get_matches_with: sc=None
[         clap::parse::parser] 	Parser::parse_short_arg: short_arg=RawOsStr("f")
[         clap::parse::parser] 	Parser::parse_short_arg:iter:f
[         clap::parse::parser] 	Parser::parse_short_arg:iter:f: cur_idx:=1
[         clap::parse::parser] 	Parser::parse_short_arg:iter:f: Found valid opt or flag
[         clap::parse::parser] 	Parser::parse_short_arg:iter:f: val=RawOsStr("") (bytes), val=[] (ascii), short_arg=RawOsStr("f")
[         clap::parse::parser] 	Parser::parse_opt; opt=foo, val=None
[         clap::parse::parser] 	Parser::parse_opt; opt.settings=ArgFlags(TAKES_VAL)
[         clap::parse::parser] 	Parser::parse_opt; Checking for val...
[         clap::parse::parser] 	Parser::parse_opt: More arg vals required...
[    clap::parse::arg_matcher] 	ArgMatcher::inc_occurrence_of: arg=foo
[            clap::build::app] 	App::groups_for_arg: id=foo
[            clap::build::app] 	App::groups_for_arg: id=foo
[         clap::parse::parser] 	Parser::get_matches_with: After parse_short_arg Opt(foo)
[         clap::parse::parser] 	Parser::get_matches_with: Begin parsing 'RawOsStr("input")' ([105, 110, 112, 117, 116])
[         clap::parse::parser] 	Parser::get_matches_with: Positional counter...1
[         clap::parse::parser] 	Parser::get_matches_with: Low index multiples...false
[         clap::parse::parser] 	Parser::add_val_to_arg; arg=foo, val=RawOsStr("input")
[         clap::parse::parser] 	Parser::add_val_to_arg; trailing_values=false, DontDelimTrailingVals=false
[         clap::parse::parser] 	Parser::add_single_val_to_arg: adding val..."input"
[         clap::parse::parser] 	Parser::add_single_val_to_arg: cur_idx:=2
[            clap::build::app] 	App::groups_for_arg: id=foo
[    clap::parse::arg_matcher] 	ArgMatcher::needs_more_vals: o=foo
[    clap::parse::arg_matcher] 	ArgMatcher::needs_more_vals: max_vals...1
[         clap::parse::parser] 	Parser::remove_overrides
[         clap::parse::parser] 	Parser::remove_overrides:iter:foo
[      clap::parse::validator] 	Validator::validate
[         clap::parse::parser] 	Parser::add_env: Checking arg `--help`
[         clap::parse::parser] 	Parser::add_env: Skipping existing arg `--foo <FOO>`
[         clap::parse::parser] 	Parser::add_env: Checking arg `<INPUT>`
[         clap::parse::parser] 	Parser::add_defaults
[         clap::parse::parser] 	Parser::add_defaults:iter:foo:
[         clap::parse::parser] 	Parser::add_value: doesn't have conditional defaults
[         clap::parse::parser] 	Parser::add_value:iter:foo: doesn't have default vals
[         clap::parse::parser] 	Parser::add_value:iter:foo: doesn't have default missing vals
[         clap::parse::parser] 	Parser::add_defaults:iter:input:
[         clap::parse::parser] 	Parser::add_value: doesn't have conditional defaults
[         clap::parse::parser] 	Parser::add_value:iter:input: doesn't have default vals
[         clap::parse::parser] 	Parser::add_value:iter:input: doesn't have default missing vals
[      clap::parse::validator] 	Validator::validate_conflicts
[      clap::parse::validator] 	Validator::validate_exclusive
[      clap::parse::validator] 	Validator::validate_exclusive:iter:foo
[      clap::parse::validator] 	Validator::gather_conflicts
[      clap::parse::validator] 	Validator::gather_conflicts:iter: id=foo
[            clap::build::app] 	App::groups_for_arg: id=foo
[      clap::parse::validator] 	Validator::validate_required: required=ChildGraph([Child { id: input, children: [] }])
[      clap::parse::validator] 	Validator::gather_requirements
[      clap::parse::validator] 	Validator::gather_requirements:iter:foo
[      clap::parse::validator] 	Validator::validate_required:iter:aog=input
[      clap::parse::validator] 	Validator::validate_required:iter: This is an arg
[      clap::parse::validator] 	Validator::is_missing_required_ok: input
[      clap::parse::validator] 	Validator::validate_arg_conflicts: a="input"
[      clap::parse::validator] 	Validator::missing_required_error; incl=[]
[      clap::parse::validator] 	Validator::missing_required_error: reqs=ChildGraph([Child { id: input, children: [] }])
[         clap::output::usage] 	Usage::get_required_usage_from: incls=[], matcher=true, incl_last=true
[         clap::output::usage] 	Usage::get_required_usage_from: unrolled_reqs={input}
[         clap::output::usage] 	Usage::get_required_usage_from:iter:input
[         clap::output::usage] 	Usage::get_required_usage_from: ret_val=["<INPUT>"]
[      clap::parse::validator] 	Validator::missing_required_error: req_args=[
    "<INPUT>",
]
[         clap::output::usage] 	Usage::create_usage_with_title
[         clap::output::usage] 	Usage::create_usage_no_title
[         clap::output::usage] 	Usage::create_smart_usage
[         clap::output::usage] 	Usage::get_required_usage_from: incls=[foo], matcher=false, incl_last=true
[         clap::output::usage] 	Usage::get_required_usage_from: unrolled_reqs={input}
[         clap::output::usage] 	Usage::get_required_usage_from:iter:foo
[         clap::output::usage] 	Usage::get_required_usage_from:iter:input
[         clap::output::usage] 	Usage::get_required_usage_from: ret_val=["--foo <FOO>", "<INPUT>"]
[            clap::build::app] 	App::color: Color setting...
[            clap::build::app] 	Auto
[           clap::output::fmt] 	is_a_tty: stderr=true

mkayaalp avatar Nov 15 '21 19:11 mkayaalp