Allow short options to not accept implicitly attached values (-sfoo)

[zohnannor]

Please complete the following tasks

• I have searched the discussions
• I have searched the open and rejected issues

Rust Version

rustc 1.65.0 (897e37553 2022-11-02)

Clap Version

4.0.26

Minimal reproducible code

/// Slightly modified pacman example: https://github.com/clap-rs/clap/blob/f5dcfc5bd92ff5bd52c1d74f202d2565e0f3102f/examples/pacman.rs

use clap::{Arg, ArgAction, Command};

fn main() {
    let matches = Command::new("pacman")
        .subcommand(
            Command::new("query")
                .short_flag('Q')
                .long_flag("query")
                .arg(
                    Arg::new("search")
                        .short('s')
                        .long("search")
                        .conflicts_with("info")
                        .action(ArgAction::Set)
                        .num_args(1..),
                )
                .arg(
                    Arg::new("info")
                        .long("info")
                        .short('i')
                        .conflicts_with("search")
                        .action(ArgAction::Set)
                        .num_args(1..),
                ),
        )
        .subcommand(
            Command::new("sync")
                .short_flag('S')
                .long_flag("sync")
                .arg(
                    Arg::new("search")
                        .short('s')
                        .long("search")
                        .conflicts_with("info")
                        .action(ArgAction::Set)
                        .num_args(1..),
                )
                .arg(
                    Arg::new("info")
                        .long("info")
                        .conflicts_with("search")
                        .short('i')
                        .action(ArgAction::SetTrue),
                ),
        )
        .get_matches();

    dbg!(matches.subcommand());
}

Steps to reproduce the bug with the above code

cargo r --example pacman -- -Ssi

Actual Behaviour

The above command arguments (-Ssi) are interpreted as -S -s i, the s flag takes i as an argument.

Expected Behaviour

This behavior did confuse me honestly. I expected same output as from -Sis test:

error: The argument '--info' cannot be used with '--search <search>...'

Usage: pacman {sync|--sync|-S} --info

For more information try '--help'

(To be fair, I also expected -Sis to fail, but it errors with error: The argument '--search <search>...' requires a value but none was supplied, I think it should first check for conflicts and then try to parse flag's params.)

Additional Context

Command with these args	Works?	Should?	Notes
-Si	Yes	Yes ✅
-Ss	No	No ✅	Errors with The argument '--search <search>...' requires a value but none was supplied
-Sis	No	No ✅	Errors with The argument '--search <search>...' requires a value but none was supplied
-Ssi	Yes	No ❌	What this issue is about
-Sis test	No	No ✅	Errors with The argument '--info' cannot be used with '--search <search>...'
-Ssi test	No	No ❔	But, here it errors with Found argument 'test' which wasn't expected, or isn't valid in this context, but really it should say The argument '--info' cannot be used with '--search <search>...'
-S -i	Yes	Yes ✅
-S -s	No	No ✅	Errors with The argument '--search <search>...' requires a value but none was supplied
-S -i -s	No	No ✅	Errors with The argument '--search <search>...' requires a value but none was supplied
-S -s -i	No	No ✅ ❔	Errors with The argument '--search <search>...' requires a value but none was supplied, which should happen with -Ssi too, it should error with The argument '--info' cannot be used with '--search <search>...'

I apologize for possibly incorrect terminology in the issue title.

Debug Output

cargo r --features debug --example pacman -- -Ssi test
   Compiling adler v1.0.2
   Compiling gimli v0.26.2
   Compiling object v0.29.0
   Compiling rustc-demangle v0.1.21
   Compiling miniz_oxide v0.5.3
   Compiling addr2line v0.17.0
   Compiling backtrace v0.3.66
   Compiling clap v4.0.26 (/home/zohnannor/clap)
    Finished dev [unoptimized + debuginfo] target(s) in 11.34s
     Running `target/debug/examples/pacman -Ssi test`
[      clap::builder::command] 	Command::_do_parse
[      clap::builder::command] 	Command::_build: name="pacman"
[      clap::builder::command] 	Command::_propagate:pacman
[      clap::builder::command] 	Command::_check_help_and_version:pacman expand_help_tree=false
[      clap::builder::command] 	Command::long_help_exists
[      clap::builder::command] 	Command::_check_help_and_version: Building default --help
[      clap::builder::command] 	Command::_check_help_and_version: Building help subcommand
[      clap::builder::command] 	Command::_propagate_global_args:pacman
[clap::builder::debug_asserts] 	Command::_debug_asserts
[clap::builder::debug_asserts] 	Arg::_debug_asserts:help
[clap::builder::debug_asserts] 	Command::_verify_positionals
[        clap::parser::parser] 	Parser::get_matches_with
[        clap::parser::parser] 	Parser::get_matches_with: Begin parsing 'RawOsStr("-Ssi")' ([45, 83, 115, 105])
[        clap::parser::parser] 	Parser::possible_subcommand: arg=Ok("-Ssi")
[        clap::parser::parser] 	Parser::get_matches_with: sc=None
[        clap::parser::parser] 	Parser::parse_short_arg: short_arg=ShortFlags { inner: RawOsStr("Ssi"), utf8_prefix: CharIndices { front_offset: 0, iter: Chars(['S', 's', 'i']) }, invalid_suffix: None }
[        clap::parser::parser] 	Parser::parse_short_arg:iter:S
[        clap::parser::parser] 	Parser::parse_short_arg:iter:S: subcommand=sync
[        clap::parser::parser] 	Parser::parse_short_arg: cur_idx:=1
[        clap::parser::parser] 	Parser::get_matches_with: After parse_short_arg FlagSubCommand("sync")
[        clap::parser::parser] 	Parser::get_matches_with:FlagSubCommandShort: subcmd_name=sync, keep_state=true, flag_subcmd_skip=1
[        clap::parser::parser] 	Parser::parse_subcommand
[         clap::output::usage] 	Usage::get_required_usage_from: incls=[], matcher=false, incl_last=true
[         clap::output::usage] 	Usage::get_required_usage_from: unrolled_reqs=[]
[         clap::output::usage] 	Usage::get_required_usage_from: ret_val=[]
[      clap::builder::command] 	Command::_build_subcommand Setting bin_name of sync to "pacman sync"
[      clap::builder::command] 	Command::_build_subcommand Setting display_name of sync to "pacman-sync"
[      clap::builder::command] 	Command::_build: name="sync"
[      clap::builder::command] 	Command::_propagate:sync
[      clap::builder::command] 	Command::_check_help_and_version:sync expand_help_tree=false
[      clap::builder::command] 	Command::long_help_exists
[      clap::builder::command] 	Command::_check_help_and_version: Building default --help
[      clap::builder::command] 	Command::_propagate_global_args:sync
[clap::builder::debug_asserts] 	Command::_debug_asserts
[clap::builder::debug_asserts] 	Arg::_debug_asserts:search
[clap::builder::debug_asserts] 	Arg::_debug_asserts:info
[clap::builder::debug_asserts] 	Arg::_debug_asserts:help
[clap::builder::debug_asserts] 	Command::_verify_positionals
[        clap::parser::parser] 	Parser::parse_subcommand: About to parse sc=sync
[        clap::parser::parser] 	Parser::get_matches_with
[        clap::parser::parser] 	Parser::get_matches_with: Begin parsing 'RawOsStr("-Ssi")' ([45, 83, 115, 105])
[        clap::parser::parser] 	Parser::possible_subcommand: arg=Ok("-Ssi")
[        clap::parser::parser] 	Parser::get_matches_with: sc=None
[        clap::parser::parser] 	Parser::parse_short_arg: short_arg=ShortFlags { inner: RawOsStr("Ssi"), utf8_prefix: CharIndices { front_offset: 0, iter: Chars(['S', 's', 'i']) }, invalid_suffix: None }
[        clap::parser::parser] 	Parser::parse_short_arg:iter:s
[        clap::parser::parser] 	Parser::parse_short_arg:iter:s: Found valid opt or flag
[        clap::parser::parser] 	Parser::parse_short_arg:iter:s: val=RawOsStr("i") (bytes), val=[105] (ascii), short_arg=ShortFlags { inner: RawOsStr("Ssi"), utf8_prefix: CharIndices { front_offset: 2, iter: Chars(['i']) }, invalid_suffix: None }
[        clap::parser::parser] 	Parser::parse_opt_value; arg=search, val=Some(RawOsStr("i")), has_eq=false
[        clap::parser::parser] 	Parser::parse_opt_value; arg.settings=ArgFlags(NO_OP)
[        clap::parser::parser] 	Parser::parse_opt_value; Checking for val...
[        clap::parser::parser] 	Parser::react action=Set, identifier=Some(Short), source=CommandLine
[        clap::parser::parser] 	Parser::react: cur_idx:=2
[        clap::parser::parser] 	Parser::remove_overrides: id="search"
[   clap::parser::arg_matcher] 	ArgMatcher::start_custom_arg: id="search", source=CommandLine
[      clap::builder::command] 	Command::groups_for_arg: id="search"
[        clap::parser::parser] 	Parser::push_arg_values: ["i"]
[        clap::parser::parser] 	Parser::add_single_val_to_arg: cur_idx:=3
[   clap::parser::arg_matcher] 	ArgMatcher::needs_more_vals: o=search, pending=0
[   clap::parser::arg_matcher] 	ArgMatcher::needs_more_vals: expected=1..=18446744073709551615, actual=0
[        clap::parser::parser] 	Parser::react not enough values passed in, leaving it to the validator to complain
[        clap::parser::parser] 	Parser::get_matches_with: After parse_short_arg ValuesDone
[        clap::parser::parser] 	Parser::get_matches_with: Begin parsing 'RawOsStr("test")' ([116, 101, 115, 116])
[        clap::parser::parser] 	Parser::possible_subcommand: arg=Ok("test")
[        clap::parser::parser] 	Parser::get_matches_with: sc=None
[        clap::parser::parser] 	Parser::get_matches_with: Positional counter...1
[        clap::parser::parser] 	Parser::get_matches_with: Low index multiples...false
[         clap::output::usage] 	Usage::create_usage_with_title
[         clap::output::usage] 	Usage::create_usage_no_title
[         clap::output::usage] 	Usage::create_help_usage; incl_reqs=true
[         clap::output::usage] 	Usage::needs_options_tag
[         clap::output::usage] 	Usage::needs_options_tag:iter: f=search
[      clap::builder::command] 	Command::groups_for_arg: id="search"
[         clap::output::usage] 	Usage::needs_options_tag:iter: [OPTIONS] required
[         clap::output::usage] 	Usage::get_args: incls=[]
[         clap::output::usage] 	Usage::get_args: unrolled_reqs=[]
[         clap::output::usage] 	Usage::get_args: ret_val=[]
[         clap::output::usage] 	Usage::create_help_usage: usage=pacman {sync|--sync|-S} [OPTIONS]
[      clap::builder::command] 	Command::color: Color setting...
[      clap::builder::command] 	Auto
[      clap::builder::command] 	Command::color: Color setting...
[      clap::builder::command] 	Auto
error: Found argument 'test' which wasn't expected, or isn't valid in this context

Usage: pacman {sync|--sync|-S} [OPTIONS]

For more information try '--help'

[epage]

The above command arguments (-Ssi) are interpreted as -S -s i, the s flag takes i as an argument.

-s takes a value and short flags are allowed to have an implicitly attached value, so this is expected. I think you can set requires_equals to bypass the implicitly attached value but that will require an = always and not just disable implicitly attached values.

[zohnannor]

Yes, it is working as it is written in the code, as expected. What I'm saying is (I guess, C-bug Category: Updating dependencies label is incorrect here, sorry), shouldn't there be at least an option to forbid attached values for arguments? Also, regarding -S -s -i, shouldn't it check for conflicts first? pacman itself works like that:

$ sudo pacman -Sis
error: invalid option: '--info' and '--search' may not be used together
$ sudo pacman -Ssi
error: invalid option: '--info' and '--search' may not be used together
$ sudo pacman -S -s -i
error: invalid option: '--info' and '--search' may not be used together

[epage]

shouldn't there be at least an option to forbid attached values for arguments?

Unsure, will have to give it some thought. It seems like some more flexibility around implicitly attached, explicitly attached, and detached values could help with some cases. See also #3030

[epage]

btw by disallowing attached values, you might not get

error: The argument '--info' cannot be used with '--search <search>...'

Usage: pacman {sync|--sync|-S} --info

For more information try '--help'

but instead an error about --search missing a value

[zohnannor]

Yes, and I've also mentioned that in the table for entry -S -s -i, it is counter-intuitive and possibly even deserves its own issue. I presume it depends on the order in which clap parses arguments and it would be a breaking change to modify it.