diff --git a/CHANGELOG.rst b/CHANGELOG.rst index fb45bde82..b38d70076 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -45,6 +45,7 @@ Scripting improvements - ``argparse`` now saves recognised options and values in ``$argv_opts``, allowing them to be forwarded to other commands (:issue:`6466`). - ``argparse`` options can now be marked to be deleted from ``$argv_opts`` (by adding a ``&`` at the end of the option spec, before a ``!`` if present). There is now also a corresponding ``-d`` / ``--delete`` option to ``fish_opt``. - ``argparse --ignore-unknown`` now removes preceding known short options from groups containing unknown options (e.g. when parsing ``-abc``, if ``a`` is known but ``b`` is not, then ``$argv`` will contain ``-bc``). +- ``argparse`` now has an ``-u`` / ``--move-unknown`` option that works like ``--ignore-unknown``, but unknown options (and their arguments) are moved from ``$argv`` to ``$argv_opts``. whereas ``--ignore-unknown`` keeps them in ``$argv``. Interactive improvements ------------------------ diff --git a/doc_src/cmds/argparse.rst b/doc_src/cmds/argparse.rst index 017b58140..c007ce5ab 100644 --- a/doc_src/cmds/argparse.rst +++ b/doc_src/cmds/argparse.rst @@ -40,12 +40,15 @@ The following ``argparse`` options are available. They must appear before all *O **-X** or **--max-args** *NUMBER* The maximum number of acceptable non-option arguments. The default is infinity. -**-i** or **--ignore-unknown** - Ignore unknown options, keeping them and their arguments in ``$argv`` instead and not moving them to ``$argv_opts``. Unknown options are treated as if they take optional arguments (i.e. have option spec ``=?``). +**-u** or **--move-unknown** + Allow unknown options, and move them from ``$argv`` to ``$argv_opts``. Unknown options are treated as if they take optional arguments (i.e. have option spec ``=?``). The above means that if a group of short options contains an unknown short option *followed* by a known short option, the known short option is - treated as an argument to the unknown one (e.g. ``--ignore-unknown h -- -oh`` will treat ``h`` as the argument to ``-o``, and so ``_flag_h`` will *not* be set). - In contrast, if the known option comes first (and does not take any arguments), the known option will be recognised and moved from ``$argv`` to ``$argv_opts`` (e.g. ``argparse --ignore-unknown h -- -ho`` will set ``$argv`` to ``-o`` and ``$argv_opts`` to ``-h``) + treated as an argument to the unknown one (e.g. ``--move-unknown h -- -oh`` will treat ``h`` as the argument to ``-o``, and so ``_flag_h`` will *not* be set). + In contrast, if the known option comes first (and does not take any arguments), the known option will be recognised (e.g. ``argparse --move-unknown h -- -ho`` *will* set ``$_flag_h`` to ``-h``) + +**-i** or **--ignore-unknown** + Deprecated. This is like **--move-unknown**, except that unknown options and their arguments are kept in ``$argv`` and not moved to ``$argv_opts``. Unlike **--move-unknown**, this option makes it impossible to distinguish between an unknown option and non-option argument that starts with a ``-`` (since any ``--`` seperator in ``$argv`` will be removed). **-s** or **--stop-nonopt** Causes scanning the arguments to stop as soon as the first non-option argument is seen. Among other things, this is useful to implement subcommands that have their own options. @@ -95,7 +98,7 @@ But this is not:: set -l argv argparse 'h/help' 'n/name' $argv -The first ``--`` seen is what allows the ``argparse`` command to reliably separate the option specifications and options to ``argparse`` itself (like ``--ignore-unknown``) from the command arguments, so it is required. +The first ``--`` seen is what allows the ``argparse`` command to reliably separate the option specifications and options to ``argparse`` itself (like ``--move-unknown``) from the command arguments, so it is required. .. _cmd-argparse-option-specification: @@ -274,8 +277,7 @@ An example of using ``$argv_opts`` to forward known options to another command, set -l opt_spec n/lines= q/quiet silent v/verbose z/zero-terminated help version # --qwords is a new option, but --bytes is an existing one which we will modify below set -a opt_spec "qwords=&" "c/bytes=&" - argparse $opt_spec -- $argv || return - + argparse --move-unknown $opt_spec -- $argv || return if set -q _flag_qwords # --qwords allows specifying the size in multiples of 8 bytes set -a argv_opts --bytes=(math -- $_flag_qwords \* 8 || return) @@ -302,4 +304,4 @@ An example of using ``$argv_opts`` to forward known options to another command, The argparse call above saves all the options we do *not* want to process in ``$argv_opts``. (The ``--qwords`` and ``--bytes`` options are *not* saved there as their option spec's end in a ``~``). The code then processes the ``--qwords`` and ``--bytess`` options using the the ``$_flag_OPTION`` variables, and puts the transformed options in ``$argv_opts`` (which already contains all the original options, *other* than ``--qwords`` and ``--bytes``). -Note that the ``argparse`` call does need to know all the options to the original ``head`` so that we can accurately work out the *non*-option arguments (i.e. ``$argv``, the filenames that ``head`` is to operate on). We'd similarly need to tell ``argparse`` the options if we wanted to modify the given filenames. +Note that because the ``argparse`` call above uses ``--move-unknown``, if the original ``head`` adds any new options, the wrapper script can still be used; however, such new options must have their arguments given in "stuck" form (e.g. ``-o`` or ``--opt=``). This is needed for the wrapper script to work out the *non*-option arguments (i.e. ``$argv``, the filenames that ``head`` is to operate on). diff --git a/share/completions/argparse.fish b/share/completions/argparse.fish index e5400be0e..c76421b2f 100644 --- a/share/completions/argparse.fish +++ b/share/completions/argparse.fish @@ -67,6 +67,10 @@ complete --command argparse --short-option N --long-option min-args --no-files - complete --command argparse --short-option X --long-option max-args --no-files --require-parameter \ --description 'Specify maximum non-option argument count' complete --command argparse --short-option i --long-option ignore-unknown \ + -n '! __fish_seen_argument --short u --long move-unknown' \ --description 'Ignore unknown options' +complete --command argparse --short-option u --long-option move-unknown \ + -n '! __fish_seen_argument --short i --long ignore-unknown' \ + --description 'Move unknown options into \$argv_opts' complete --command argparse --short-option s --long-option stop-nonopt \ --description 'Exit on subcommand' diff --git a/src/builtins/argparse.rs b/src/builtins/argparse.rs index 0ec1f4e5e..1a5b388ba 100644 --- a/src/builtins/argparse.rs +++ b/src/builtins/argparse.rs @@ -49,9 +49,17 @@ fn new(s: char) -> Self { } } +#[derive(Default, PartialEq)] +enum UnknownHandling { + #[default] + Error, + Ignore, + Move, +} + #[derive(Default)] struct ArgParseCmdOpts<'args> { - ignore_unknown: bool, + unknown_handling: UnknownHandling, print_help: bool, stop_nonopt: bool, min_args: usize, @@ -75,10 +83,11 @@ fn new() -> Self { } } -const SHORT_OPTIONS: &wstr = L!("+:hn:six:N:X:"); +const SHORT_OPTIONS: &wstr = L!("+:hn:siux:N:X:"); const LONG_OPTIONS: &[WOption] = &[ wopt(L!("stop-nonopt"), ArgType::NoArgument, 's'), wopt(L!("ignore-unknown"), ArgType::NoArgument, 'i'), + wopt(L!("move-unknown"), ArgType::NoArgument, 'u'), wopt(L!("name"), ArgType::RequiredArgument, 'n'), wopt(L!("exclusive"), ArgType::RequiredArgument, 'x'), wopt(L!("help"), ArgType::NoArgument, 'h'), @@ -507,7 +516,22 @@ fn parse_cmd_opts<'args>( match c { 'n' => opts.name = w.woptarg.unwrap().to_owned(), 's' => opts.stop_nonopt = true, - 'i' => opts.ignore_unknown = true, + 'i' | 'u' => { + if opts.unknown_handling != UnknownHandling::Error { + streams.err.append(wgettext_fmt!( + BUILTIN_ERR_COMBO2_EXCLUSIVE, + cmd, + "--ignore-unknown", + "--move-unknown" + )); + return Err(STATUS_INVALID_ARGS); + }; + opts.unknown_handling = if c == 'i' { + UnknownHandling::Ignore + } else { + UnknownHandling::Move + } + } // Just save the raw string here. Later, when we have all the short and long flag // definitions we'll parse these strings into a more useful data structure. 'x' => opts.raw_exclusive_flags.push(w.woptarg.unwrap()), @@ -865,7 +889,7 @@ fn argparse_parse_flags<'args>( is_long_flag, streams, ) - } else if !opts.ignore_unknown { + } else if opts.unknown_handling == UnknownHandling::Error { streams.err.append(wgettext_fmt!( BUILTIN_ERR_UNKNOWN, opts.name, @@ -891,12 +915,22 @@ fn argparse_parse_flags<'args>( w.remaining_text = L!(""); } - // Now by calling delete_flag we ensure that if the unknown flag is precceded by - // known flags, the known flags are kept in $argv_opts, and not added to $argv. - // (any argument to the option is also returned in unknown_flag, and removed - // from opts.argv_opts) - let unknown_flag = delete_flag(&mut w, is_long_flag); - opts.args.push(unknown_flag); + if opts.unknown_handling == UnknownHandling::Ignore { + // Now by calling delete_flag we ensure that if the unknown flag is + // precceded by known flags, the known flags are kept in $argv_opts, and not + // added to $argv. (any argument to the option is also returned in + // unknown_flag, and removed from opts.argv_opts) Now by calling delete_flag + // we ensure that if the unknown flag is precceded by known flags, the known + // flags are kept in $argv_opts, and not added to $argv. (any argument to + // the option is also returned in unknown_flag, and removed from + // opts.argv_opts) + let unknown_flag = delete_flag(&mut w, is_long_flag); + opts.args.push(unknown_flag); + } else { + assert!(opts.unknown_handling == UnknownHandling::Move); + // Nothing more to do, w.argv_opts will already contain the option, and its + // value (if any) + } // Work around weirdness with wgetopt, which crashes if we `continue` here. if w.wopt_index == argc { diff --git a/tests/checks/argparse.fish b/tests/checks/argparse.fish index 462c44a6b..c63a53853 100644 --- a/tests/checks/argparse.fish +++ b/tests/checks/argparse.fish @@ -368,6 +368,62 @@ begin # CHECK: argv_opts -b end +begin + # Ignore unknown and move unknown are mutually exclusive + argparse -i --move-unknown -- + #CHECKERR: argparse: --ignore-unknown --move-unknown: options cannot be used together + #CHECKERR: {{.*}}checks/argparse.fish (line {{\d+}}): + #CHECKERR: argparse -i --move-unknown -- + #CHECKERR: ^ + #CHECKERR: (Type 'help argparse' for related documentation) +end + +begin + # Moving unknown options + argparse --move-unknown h i -- -hoa -oia + echo -- $argv + #CHECK: + echo -- $argv_opts + #CHECK: -hoa -oia + echo $_flag_h + #CHECK: -h + set -q _flag_i + or echo No flag I + #CHECK: No flag I +end + +begin + # Moving unknown options + argparse -u a=+ b=+ -- -a alpha -b bravo -t tango -a aaaa --wurst + or echo unexpected argparse return status $status >&2 + # The unknown options are removed _entirely_. + echo $argv + echo $argv_opts + echo $_flag_a + # CHECK: tango + # CHECK: -a alpha -b bravo -t -a aaaa --wurst + # CHECK: alpha aaaa +end + +begin + # Move unknown long options that share a character with a known short options + argparse -u o -- --long=value --long + or echo unexpected argparse return status $status >&2 + set -l + # CHECK: argv + # CHECK: argv_opts '--long=value' '--long' +end + + +begin + argparse -u b/break -- "-b kubectl get pods -l name=foo" + set -l + # CHECK: _flag_b -b + # CHECK: _flag_break -b + # CHECK: argv + # CHECK: argv_opts '-b kubectl get pods -l name=foo' +end + begin # Checking arguments after "--" argparse a/alpha -- a --alpha -- b -a