Added a -U/--unknown-arguments option to argparse

The new -U/--unknown-arguments option takes either 'optional', 'required', or
'none', indicating how many arguments unknown options are assumed to take.
The default is optional, the same behaviour as before this commit, despite
most options in practice taking not taking any arguments. Using
--unknown-arguments=required and --unknown-arguments=none (but not
--unknown-arguments=optional) can give you parse errors if, respectively,
an unknown option has no argument (because it the option is at the end of the
argument list), or is given an argument (with the `--flag=<value> syntax).
See doc_src/cmds/argparse.rst for more details (specifically, the descritpion
of the --unknown-arguments flag and the example at the end
of the examples section).

As a convenience, -U/--unknown-arguments implies -u/--move-unknown.
However you can use it the deprecated -i/--ignore-unknown if you really want to.

CHANGELOG.rst

@@ -47,6 +47,7 @@ Scripting improvements
 - ``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``.
 - ``argparse`` now has an ``-S`` / ``--strict-longopts`` option that forbids abbreviating long options or passing them with a single dash (e.g. if there is a long option called ``foo``, ``--fo`` and ``--foo`` won't match it).
+- ``argparse`` now has a ``-U`` / ``--unknown-arguments`` *KIND* option, where *KIND* is either ``optional``, ``required``, or ``none``, indicating whether unknown options are parsed as taking optional, required, or no arguments. This implies ``--move-unknown``.
 
 Interactive improvements
 ------------------------

doc_src/cmds/argparse.rst

@@ -41,7 +41,7 @@ The following ``argparse`` options are available. They must appear before all *O
     The maximum number of acceptable non-option arguments. The default is infinity.
 
 **-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 ``=?``).
+    Allow unknown options, and move them from ``$argv`` to ``$argv_opts``. By default, 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. ``--move-unknown h -- -oh`` will treat ``h`` as the argument to ``-o``, and so ``_flag_h`` will *not* be set).
@@ -65,6 +65,20 @@ The following ``argparse`` options are available. They must appear before all *O
 
     This option may be on all the time in the future, so do not rely on the behaviour without it.
 
+**--unknown-arguments** *KIND*
+    This option implies **--move-unknown**, unless **--ignore-unknown** is also given.
+    This will modify the parsing behaviour of unknown options depending on the value of *KIND*:
+
+        - **optional** (the default), allows each unknown option to take an optional argument (i.e. as if it had ``=?`` in its option specification). For example, ``argparse --ignore-unknown --unknown-arguments=optional ab -- -u -a -ub`` will set ``_flag_a`` but *not* ``_flag_b``, as the ``b`` is treated as an argument to the second use of ``-u``.
+
+        - **required** requires each unknown option to take an argument (i.e. as if it had ``=`` or ``=+`` in its option specification). If the above example was changed to use ``--unknown-arguments=required``, *neither* ``_flag_a`` nor ``_flag_b`` would be set: the ``-a`` will be treated as an argument to the first use of ``-u``, and the ``b`` as an argument to the second.
+
+        - **none** forbids each unknown option from taking an argument (i.e. as if it had no ``=`` in its option specification). If the above example was changed to use ``--unknown-arguments=none``, *both* ``_flag_a`` and ``_flag_b`` would be set, as neither use of ``-u`` will be passed as taking an argument.
+
+        Note that the above assumes that unknown long flags use the ``--`` "GNU-style" (e.g. if *KIND* is ``none``, and there is no ``bar`` long option, ``-bar`` is interpreted as three short flags, ``b``, ``a``, and ``r``; but if ``bar`` is known, ``-bar`` is treated the same as ``--bar``).
+
+        When using ``--unknown-arguments=required``, you will get an error if the provided arguments end in an unknown option, since it has no argument. Similarly, with ``--unknown-arguments=none``, you will get an error if you use the ``--flag=value`` syntax and ``flag`` is an unknown option.
+
 **-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.
 
@@ -288,11 +302,12 @@ After this it figures out which variable it should operate on according to the `
 An example of using ``$argv_opts`` to forward known options to another command, whilst adding new options::
 
     function my-head
-        # The following options are existing ones to head that we will forward verbatim
-        set -l opt_spec n/lines= q/quiet silent v/verbose z/zero-terminated help version
+        # The following option is the only existing one to head that takes arguments
+        # (we will forward it verbatim).
+        set -l opt_spec n/lines=
         # --qwords is a new option, but --bytes is an existing one which we will modify below
         set -a opt_spec "qwords=&" "c/bytes=&"
-        argparse --move-unknown $opt_spec -- $argv || return
+        argparse --strict-longopts --move-unknown --unknown-arguments=none $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)
@@ -319,4 +334,6 @@ 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 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<arg>`` or ``--opt=<arg>``). 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).
+Note that because the ``argparse`` call above uses ``--move-unknown`` and ``--unknown-arguments=none``, we only need to tell it the arguments to ``head`` that take a value. This allows the wrapper script to accurately work out the *non*-option arguments (i.e. ``$argv``, the filenames that ``head`` is to operate on). Using ``--unknown-arguments=optional`` and explicitly listing all the known options to ``head`` however would have the advantage that if ``head`` were to add new options, they could still be used with the wrapper script using the "stuck" form for arguments (e.g. ``-o<arg>``, or ``--opt=<arg>``).
+
+Note that the ``--strict-longopts`` is required to be able to correctly pass short options, e.g. without it ``my-head -q --bytes 10q``, will actually parse the ``-q`` as shorthand for ``--qwords``.

po/de.po

@@ -413,6 +413,11 @@ msgstr ""
 msgid "%ls: Invalid --min-args value '%ls'\n"
 msgstr ""
 
+#
+#, c-format
+msgid "%ls: Invalid --unknown-arguments value '%ls'\n"
+msgstr ""
+
 #, c-format
 msgid "%ls: Invalid count value '%ls'\n"
 msgstr "%ls: Ungültiger 'count'-Wert '%ls'\n"

po/en.po

@@ -411,6 +411,11 @@ msgstr ""
 msgid "%ls: Invalid --min-args value '%ls'\n"
 msgstr ""
 
+#
+#, c-format
+msgid "%ls: Invalid --unknown-arguments value '%ls'\n"
+msgstr ""
+
 #, c-format
 msgid "%ls: Invalid count value '%ls'\n"
 msgstr ""

po/fr.po

@@ -512,6 +512,11 @@ msgstr "%ls : Valeur --max-args '%ls' invalide\n"
 msgid "%ls: Invalid --min-args value '%ls'\n"
 msgstr "%ls : Valeur --min-args '%ls' invalide\n"
 
+#
+#, c-format
+msgid "%ls: Invalid --unknown-arguments value '%ls'\n"
+msgstr ""
+
 #, c-format
 msgid "%ls: Invalid count value '%ls'\n"
 msgstr "%ls : compte '%ls' invalide\n"

po/pl.po

@@ -407,6 +407,11 @@ msgstr ""
 msgid "%ls: Invalid --min-args value '%ls'\n"
 msgstr ""
 
+#
+#, c-format
+msgid "%ls: Invalid --unknown-arguments value '%ls'\n"
+msgstr ""
+
 #, c-format
 msgid "%ls: Invalid count value '%ls'\n"
 msgstr ""

po/pt_BR.po

@@ -412,6 +412,11 @@ msgstr ""
 msgid "%ls: Invalid --min-args value '%ls'\n"
 msgstr ""
 
+#
+#, c-format
+msgid "%ls: Invalid --unknown-arguments value '%ls'\n"
+msgstr ""
+
 #, c-format
 msgid "%ls: Invalid count value '%ls'\n"
 msgstr ""

po/sv.po

@@ -408,6 +408,11 @@ msgstr ""
 msgid "%ls: Invalid --min-args value '%ls'\n"
 msgstr ""
 
+#
+#, c-format
+msgid "%ls: Invalid --unknown-arguments value '%ls'\n"
+msgstr ""
+
 #, c-format
 msgid "%ls: Invalid count value '%ls'\n"
 msgstr ""

po/zh_CN.po

@@ -405,6 +405,11 @@ msgstr "%ls: 无效的 --max-args 值 '%ls'\n"
 msgid "%ls: Invalid --min-args value '%ls'\n"
 msgstr "%ls: 无效的 --min-args 值 '%ls'\n"
 
+#
+#, c-format
+msgid "%ls: Invalid --unknown-arguments value '%ls'\n"
+msgstr ""
+
 #, fuzzy, c-format
 msgid "%ls: Invalid count value '%ls'\n"
 msgstr "%s: 无效的计数值 '%s'\n"

share/completions/argparse.fish

@@ -74,5 +74,8 @@ complete --command argparse --short-option u --long-option move-unknown \
     --description 'Move unknown options into \$argv_opts'
 complete --command argparse --short-option S --long-option strict-longopts \
     --description 'Pass long options strictly'
+complete --command argparse --short-option U --long-option unknown-arguments --no-files --require-parameter \
+    --arguments "optional required none" \
+    --description 'Whether unknown options can have arguments'
 complete --command argparse --short-option s --long-option stop-nonopt \
     --description 'Exit on subcommand'

src/builtins/argparse.rs

@@ -49,6 +49,7 @@ enum UnknownHandling {
 #[derive(Default)]
 struct ArgParseCmdOpts<'args> {
     unknown_handling: UnknownHandling,
+    unknown_arguments: ArgType,
     strict_long_opts: bool,
     print_help: bool,
     stop_nonopt: bool,
@@ -68,16 +69,18 @@ impl ArgParseCmdOpts<'_> {
     fn new() -> Self {
         Self {
             max_args: usize::MAX,
+            unknown_arguments: ArgType::OptionalArgument,
             ..Default::default()
         }
     }
 }
 
-const SHORT_OPTIONS: &wstr = L!("+hn:siux:SN:X:");
+const SHORT_OPTIONS: &wstr = L!("+hn:siuU:x:SN: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!("unknown-arguments"), ArgType::RequiredArgument, 'U'),
     wopt(L!("name"), ArgType::RequiredArgument, 'n'),
     wopt(L!("exclusive"), ArgType::RequiredArgument, 'x'),
     wopt(L!("strict-longopts"), ArgType::NoArgument, 'S'),
@@ -506,6 +509,7 @@ fn parse_cmd_opts<'args>(
     let mut args_read = Vec::with_capacity(args.len());
     args_read.extend_from_slice(args);
 
+    let mut seen_unknown_arguments = false;
     let mut w = WGetopter::new(SHORT_OPTIONS, LONG_OPTIONS, args);
     while let Some(c) = w.next_opt() {
         match c {
@@ -527,6 +531,24 @@ fn parse_cmd_opts<'args>(
                     UnknownHandling::Move
                 }
             }
+            'U' => {
+                seen_unknown_arguments = true;
+                let kind = w.woptarg.unwrap();
+                opts.unknown_arguments = if kind == L!("optional") {
+                    ArgType::OptionalArgument
+                } else if kind == L!("required") {
+                    ArgType::RequiredArgument
+                } else if kind == L!("none") {
+                    ArgType::NoArgument
+                } else {
+                    streams.err.append(wgettext_fmt!(
+                        "%ls: Invalid --unknown-arguments value '%ls'\n",
+                        cmd,
+                        kind
+                    ));
+                    return Err(STATUS_INVALID_ARGS);
+                }
+            }
             'S' => opts.strict_long_opts = true,
             // 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.
@@ -588,6 +610,11 @@ fn parse_cmd_opts<'args>(
         }
     }
 
+    // Imply --unknown-arguments implies --move-unknown, unless --ignore-unknown was given
+    if seen_unknown_arguments && opts.unknown_handling == UnknownHandling::Error {
+        opts.unknown_handling = UnknownHandling::Move;
+    }
+
     if opts.print_help {
         return Ok(SUCCESS);
     }
@@ -866,7 +893,7 @@ fn argparse_parse_flags<'args>(
     w.strict_long_opts = opts.strict_long_opts;
     while let Some((opt, longopt_idx)) = w.next_opt_indexed() {
         let is_long_flag = longopt_idx.is_some();
-        let retval: BuiltinResult = match opt {
+        match opt {
             ':' => {
                 builtin_missing_argument(
                     parser,
@@ -875,7 +902,7 @@ fn argparse_parse_flags<'args>(
                     args_read[w.wopt_index - 1],
                     false,
                 );
-                Err(STATUS_INVALID_ARGS)
+                return Err(STATUS_INVALID_ARGS);
             }
             ';' => {
                 builtin_unexpected_argument(
@@ -885,7 +912,7 @@ fn argparse_parse_flags<'args>(
                     args_read[w.wopt_index - 1],
                     false,
                 );
-                Err(STATUS_INVALID_ARGS)
+                return Err(STATUS_INVALID_ARGS);
             }
             '?' => {
                 // It's not a recognized flag. See if it's an implicit int flag.
@@ -899,26 +926,46 @@ fn argparse_parse_flags<'args>(
                         &mut w,
                         is_long_flag,
                         streams,
-                    )
+                    )?;
                 } else if opts.unknown_handling == UnknownHandling::Error {
                     streams.err.append(wgettext_fmt!(
                         BUILTIN_ERR_UNKNOWN,
                         opts.name,
                         args_read[w.wopt_index - 1]
                     ));
-                    Err(STATUS_INVALID_ARGS)
+                    return Err(STATUS_INVALID_ARGS);
                 } else {
                     // The option is unknown, so there's no long opt index it could have used
                     assert!(!is_long_flag);
                     // arg_contents already skipped over the first '-'
                     let is_long_flag = arg_contents.starts_with("-");
 
+                    if is_long_flag {
+                        // For some reason, wgetopt parses unknown long options as if they where a
+                        // short '-' flag, followed by the long flag name, interpreted either as the
+                        // value for '-' or as remaining short options, so this fixes that
+                        let rest = if let Some(value) = w.woptarg {
+                            assert!(w.remaining_text.is_empty());
+                            value
+                        } else {
+                            w.remaining_text
+                        };
+                        // The arguments was of form --<long-flag>=<value>, so extract out <value>
+                        if let Some(i) = rest.find_char('=') {
+                            w.woptarg = Some(&rest[i + 1..]);
+                        }
+                        // Ensure w.remaining_text is not misinterpreted as the value of the flag,
+                        // or as remaining short options
+                        w.remaining_text = L!("");
+                    }
+
                     // Any unrecognized option is put back if ignore_unknown is used.
                     // This allows reusing the same argv in multiple argparse calls,
                     // or just ignoring the error (e.g. in completions).
 
                     // First we consume any remaining text as if it was the option's argument
-                    if !w.remaining_text.is_empty() {
+                    if opts.unknown_arguments != ArgType::NoArgument && !w.remaining_text.is_empty()
+                    {
                         assert!(w.woptarg.is_none()); // Both don't make sense
                         w.woptarg = Some(w.remaining_text);
                         // Explain to wgetopt that we want to skip to the next arg,
@@ -926,6 +973,40 @@ fn argparse_parse_flags<'args>(
                         w.remaining_text = L!("");
                     }
 
+                    // If unknown options require arguments, but there weren't any worked out above,
+                    // take the next element of w.argv as the argument
+                    let separate_value = if opts.unknown_arguments == ArgType::RequiredArgument
+                        && w.woptarg.is_none()
+                    {
+                        if w.wopt_index < w.argv.len() {
+                            w.wopt_index += 1; // Tell wgetop to skip over the options value
+                            Some(w.argv[w.wopt_index - 1])
+                        } else {
+                            // the option is at the end of argv, so it has no argument
+                            streams.err.append(wgettext_fmt!(
+                                BUILTIN_ERR_MISSING,
+                                opts.name,
+                                args_read[w.wopt_index - 1]
+                            ));
+                            return Err(STATUS_INVALID_ARGS);
+                        }
+                    } else {
+                        None
+                    };
+
+                    // If the option uses the long flag syntax with a value (i.e. --<flag>=<value>)
+                    if opts.unknown_arguments == ArgType::NoArgument
+                        && is_long_flag
+                        && arg_contents.contains('=')
+                    {
+                        streams.err.append(wgettext_fmt!(
+                            BUILTIN_ERR_UNEXP_ARG,
+                            opts.name,
+                            args_read[w.wopt_index - 1]
+                        ));
+                        return Err(STATUS_INVALID_ARGS);
+                    }
+
                     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
@@ -937,17 +1018,29 @@ fn argparse_parse_flags<'args>(
                         // opts.argv_opts)
                         let unknown_flag = delete_flag(&mut w, is_long_flag);
                         opts.args.push(unknown_flag);
+                        // If the value of the argument was the next element of the input arguments,
+                        // record it to be stored in the new $argv value
+                        if let Some(value) = separate_value {
+                            opts.args.push(Cow::Borrowed(value));
+                        }
                     } else {
                         assert!(opts.unknown_handling == UnknownHandling::Move);
-                        // Nothing more to do, w.argv_opts will already contain the option, and its
-                        // value (if any)
+                        // w.argv_opts will already contain the option and its value, unless the
+                        // value was given as a seperate argument
+                        if let Some(value) = separate_value {
+                            w.argv_opts.push(Cow::Borrowed(value));
+                        }
+                    }
+
+                    // Make wgetopt keep reading this argument for more options
+                    if !w.remaining_text.is_empty() {
+                        w.wopt_index -= 1;
                     }
 
                     // Work around weirdness with wgetopt, which crashes if we `continue` here.
                     if w.wopt_index == argc {
                         break;
                     }
-                    Ok(SUCCESS)
                 }
             }
             NON_OPTION_CHAR => {
@@ -960,9 +1053,10 @@ fn argparse_parse_flags<'args>(
                 continue;
             }
             // It's a recognized flag.
-            _ => handle_flag(parser, opts, opt, is_long_flag, &mut w, streams),
-        };
-        retval?;
+            _ => {
+                handle_flag(parser, opts, opt, is_long_flag, &mut w, streams)?;
+            }
+        }
     }
     opts.args_opts = w.argv_opts;
 

tests/checks/argparse.fish

@@ -380,7 +380,7 @@ end
 
 begin
     # Moving unknown options
-    argparse --move-unknown h i -- -hoa -oia
+    argparse --move-unknown --unknown-arguments=optional h i -- -hoa -oia
     echo -- $argv
     #CHECK:
     echo -- $argv_opts
@@ -424,6 +424,48 @@ begin
     # CHECK: argv_opts '-b kubectl get pods -l name=foo'
 end
 
+begin
+    # Checking unknown-arguments, with errors
+    argparse -i -U none -- --long=value
+    # CHECKERR: argparse: --long=value: option does not take an argument
+    argparse -i -U required -- -u
+    # CHECKERR: argparse: -u: option requires an argument
+    argparse -i -U required -- --long
+    # CHECKERR: argparse: --long: option requires an argument
+end
+
+begin
+    argparse -U none -i b= -- -abv=val in --long between -u
+    set -l
+    # CHECK: _flag_b v=val
+    # CHECK: argv '-a'  'in'  '--long'  'between'  '-u'
+    # CHECK: argv_opts -bv=val
+end
+
+begin
+    argparse -U none b= -- -abv in --long between -u
+    set -l
+    # CHECK: _flag_b v
+    # CHECK: argv 'in'  'between'
+    # CHECK: argv_opts '-abv'  '--long'  '-u'
+end
+
+begin
+    argparse -iU required b= -- -abv -b -b --long -b -u -b
+    set -l
+    # CHECK: _flag_b -b
+    # CHECK: argv '-abv'  '--long'  '-b'  '-u'  '-b'
+    # CHECK: argv_opts '-b'  '-b'
+end
+
+begin
+    argparse -uU required b= -- -abv -b -b --long -b -u -b
+    set -l
+    # CHECK: _flag_b -b
+    # CHECK: argv
+    # CHECK: argv_opts '-abv'  '-b'  '-b'  '--long'  '-b'  '-u'  '-b'
+end
+
 begin
     # Checking arguments after "--"
     argparse a/alpha -- a --alpha -- b -a
@@ -584,6 +626,24 @@ begin
     #CHECKERR: (Type 'help argparse' for related documentation)
 end
 
+begin
+    argparse -U
+    #CHECKERR: argparse: -U: option requires an argument
+    #CHECKERR: {{.*}}checks/argparse.fish (line {{\d+}}):
+    #CHECKERR: argparse -U
+    #CHECKERR: ^
+    #CHECKERR: (Type 'help argparse' for related documentation)
+end
+
+begin
+    argparse --unknown-arguments what --
+    #CHECKERR: argparse: Invalid --unknown-arguments value 'what'
+    #CHECKERR: {{.*}}checks/argparse.fish (line {{\d+}}):
+    #CHECKERR: argparse --unknown-arguments what --
+    #CHECKERR: ^
+    #CHECKERR: (Type 'help argparse' for related documentation)
+end
+
 begin
     argparse --ignore-unknown h i -- -hoa -oia
     echo -- $argv