Added argparse support for arguments with multiple optional values.

This commit fixes #8432 by adding put =* in an option spec to indicate that the
option takes an optional value, where subsequent uses of the option accumulate
the value (so the parsing behaviour is like =?, but the _flag_ variables are
appended to like =+). If the option didn't have a value, it appends an empty
string. As an example,. long=* -- --long=1 --long will execute
set -l _flag_long 1 '' (i.e. count $_flag_long is 2), whereas with =? instead,
you'd get set -l _flag_long (i.e. count $_flag_long is 0).

As a use case, I'm aware of git clone which has a
--recurse-submodules=[<pathspec>]: if you use it without a value, it operates on
all submodules, with a value, it operates on the given submodule.

The fish_opt function will generate an =* option spec when given both the
--optional-val and --multiple-vals options (previously, doing so was an error).
fish_opt now also accepts -m as an abbreviation for --multiple-vals, to go with
the pre-existing -o and -r abbreviations for --optional-val and --required-val.

CHANGELOG.rst

@@ -48,6 +48,7 @@ Scripting improvements
 - ``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``.
+- ``argparse`` now allows specifying options that take multiple optional values by using ``=*`` in the option spec, the parsing of the option is the same as ones with optional values (i.e. ``=?``), but each successive use accumulates more values (or an empty string if no value), instead of replacing the previous value (i.e. it behaves similarly to ``=+``) (:issue:`8432`). In addition, ``fish_opt`` has been modified to support such options by using the ``--multiple-vals`` together with ``-o`` / ``--optional-val``; ``-m`` is also now acceptable as an abbreviation for ``--multiple-vals``.
 
 Interactive improvements
 ------------------------

doc_src/cmds/argparse.rst

@@ -69,7 +69,7 @@ The following ``argparse`` options are available. They must appear before all *O
     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``.
+        - **optional** (the default), allows each unknown option to take an optional argument (i.e. as if it had ``=?`` or ``=*`` 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.
 
@@ -146,7 +146,9 @@ Each option specification consists of:
 
     - **=?** if it takes an optional value and only the last instance of the flag is saved, or
 
-    - **=+** if it requires a value and each instance of the flag is saved.
+    - **=+** if it requires a value and each instance of the flag is saved, or
+
+    - **=\*** if it takes an optional value *and* each instance of the flag is saved, storing the empty string when the flag was not given a value.
 
 - Optionally a ``&``, indicating that the option and any attached values are not to be saved in ``$argv`` or ``$argv_opts``. This does not affect the the ``_flag_`` variables.
 
@@ -170,7 +172,7 @@ This does not read numbers given as ``+NNN``, only those that look like flags -
 Note: Optional arguments
 ------------------------
 
-An option defined with ``=?`` can take optional arguments. Optional arguments have to be *directly attached* to the option they belong to.
+An option defined with ``=?`` or ``=*`` can take optional arguments. Optional arguments have to be *directly attached* to the option they belong to.
 
 That means the argument will only be used for the option if you use it like::
 
@@ -245,6 +247,8 @@ Some *OPTION_SPEC* examples:
 
 - ``n/name=?`` means that both ``-n`` and ``--name`` are valid. It accepts an optional value and can be used at most once. If the flag is seen then ``_flag_n`` and ``_flag_name`` will be set with the value associated with the flag if one was provided else it will be set with no values.
 
+- ``n/name=*`` is similar, but the flag can be used more than once. If the flag is seen then ``_flag_n`` and ``_flag_name`` will be set with the values associated with each occurence. Each value will be the value given to the option, or the empty string if no value was given.
+
 - ``name=+`` means that only ``--name`` is valid. It requires a value and can be used more than once. If the flag is seen then ``_flag_name`` will be set with the values associated with each occurrence.
 
 - ``x`` means that only ``-x`` is valid. It is a boolean that can be used more than once. If it is seen then ``_flag_x`` will be set as above.

doc_src/cmds/fish_opt.rst

@@ -8,7 +8,7 @@ Synopsis
 
 .. synopsis::
 
-    fish_opt -s ALPHANUM [-l LONG-NAME] [-ord] [--multiple-vals] [--long-only]
+    fish_opt -s ALPHANUM [-l LONG-NAME] [-ormd] [--long-only]
     fish_opt --help
 
 Description
@@ -33,8 +33,8 @@ The following ``argparse`` options are available:
 **-r** or **--required-val**
     The option being defined requires a value. If the option is seen more than once when parsing arguments, only the last value seen is saved. This means the resulting flag variable created by ``argparse`` will have exactly one element.
 
-**--multiple-vals**
-    The option being defined requires a value each time it is seen. Each instance is stored. This means the resulting flag variable created by ``argparse`` will have one element for each instance of this option in the arguments.
+**-m** or **--multiple-vals**
+    The value of each instance of the option is accumulated. If **--optional-val** is provided, the value is optional, and an empty string is stored if no value is provided. Otherwise, the **--requiured-val** option is implied and each instance of the option requires a value. This means the resulting flag variable created by ``argparse`` will have one element for each instance of this option in the arguments, even for instances that did not provide a value.
 
 **-d** or **--delete**
     The option and any values will be deleted from the ``$argv_opts`` variables set by ``argparse``

share/completions/fish_opt.fish

@@ -9,5 +9,5 @@ complete --command fish_opt --short-option l --long-option long --no-files --req
 complete --command fish_opt --long-option longonly --description 'Use only long option'
 complete --command fish_opt --short-option o --long-option optional-val -n $CONDITION --description 'Don\'t require value'
 complete --command fish_opt --short-option r --long-option required-val -n $CONDITION --description 'Require value'
-complete --command fish_opt --long-option multiple-vals --description 'Store all values'
+complete --command fish_opt --short-option m --long-option multiple-vals --description 'Store all values'
 complete --command fish_opt --short-option d --long-option delete --description 'Delete option from argv_opts'

share/functions/fish_opt.fish

@@ -11,9 +11,9 @@ end
 
 # The `fish_opt` command.
 function fish_opt -d 'Produce an option specification suitable for use with `argparse`.'
-    set -l options h/help 's/short=' 'l/long=' d/delete o/optional-val r/required-val
-    set options $options L-long-only M-multiple-vals
-    argparse -n fish_opt --max-args=0 --exclusive=r,o --exclusive=M,o $options -- $argv
+    set -l options h/help 's/short=' 'l/long=' d/delete o/optional-val r/required-val m/multiple-vals
+    set options $options L-long-only
+    argparse -n fish_opt --max-args=0 --exclusive=r,o $options -- $argv
     or return
 
     if set -q _flag_help
@@ -35,7 +35,9 @@ function fish_opt -d 'Produce an option specification suitable for use with `arg
         set opt_spec "$opt_spec$_flag_long"
     end
 
-    if set -q _flag_multiple_vals
+    if set -q _flag_multiple_vals && set -q _flag_optional_val
+        set opt_spec "$opt_spec=*"
+    else if set -q _flag_multiple_vals
         set opt_spec "$opt_spec=+"
     else if set -q _flag_required_val
         set opt_spec "$opt_spec="

src/builtins/argparse.rs

@@ -234,6 +234,10 @@ fn parse_flag_modifiers<'args>(
                 s = s.slice_from(1);
                 (ArgType::RequiredArgument, true)
             }
+            '*' => {
+                s = s.slice_from(1);
+                (ArgType::OptionalArgument, true)
+            }
             _ => (ArgType::RequiredArgument, false),
         };
     }
@@ -858,7 +862,11 @@ fn handle_flag<'args>(
     }
 
     if opt_spec.accumulate_args {
-        opt_spec.vals.push(w.woptarg.unwrap().into());
+        if let Some(arg) = w.woptarg {
+            opt_spec.vals.push(arg.into());
+        } else {
+            opt_spec.vals.push(WString::new());
+        }
     } else {
         // We're depending on `next_opt()` to report that a mandatory value is missing if
         // `opt_spec->arg_type == ArgType::RequiredArgument` and thus return ':' so that we don't

tests/checks/argparse.fish

@@ -552,11 +552,6 @@ and echo unexpected status $status
 #CHECKERR: fish_opt: o/optional-val r/required-val: options cannot be used together
 # XXX FIXME the error should output -r and --optional-val: what the user used
 
-# A repeated and optional arg makes no sense
-fish_opt -s h -l help --multiple-vals --optional-val
-and echo unexpected status $status
-#CHECKERR: fish_opt: multiple-vals o/optional-val: options cannot be used together
-
 # An unexpected arg not associated with a flag is an error
 fish_opt -s h -l help hello
 and echo unexpected status $status
@@ -598,19 +593,37 @@ fish_opt --short h -l help --multiple-vals
 or echo unexpected status $status
 #CHECK: h/help=+
 
+# Repeated and optional val, short and long valid
+fish_opt --short h -l help --optional-val -m
+or echo unexpected status $status
+#CHECK: h/help=*
+
 # Repeated val, short and long but short not valid
-fish_opt --short h -l help --multiple-vals --long-only
+fish_opt --short h -ml help --long-only
 or echo unexpected status $status
 #CHECK: h-help=+
 
+# Repeated and optional val, short and long but short not valid
+fish_opt --short h -l help --long-only -mo
+or echo unexpected status $status
+#CHECK: h-help=*
+
 # Repeated val, short only, and deleted
 fish_opt -s h --multiple-vals
 or echo unexpected status $status
 #CHECK: h=+
-fish_opt -s h --multiple-vals --long-only -d
+fish_opt -s h -md --long-only
 or echo unexpected status $status
 #CHECK: h=+&
 
+# Repeated and optional val, short only
+fish_opt -s h -om
+or echo unexpected status $status
+fish_opt -s h --optional-val --multiple-vals --long-only
+or echo unexpected status $status
+#CHECK: h=*
+#CHECK: h=*
+
 function wrongargparse
     argparse -foo -- banana
     argparse a-b
@@ -735,5 +748,16 @@ begin
     # CHECKERR: argparse: -valu: unknown option
 end
 
+# Check =* options
+begin
+    set -l _flag_o previous value
+    argparse 'o/opt=*' -- -o non-value -oval --opt arg --opt=456
+    set -l
+    # CHECK: _flag_o ''  'val'  ''  '456'
+    # CHECK: _flag_opt ''  'val'  ''  '456'
+    # CHECK: argv 'non-value'  'arg'
+    # CHECK: argv_opts '-o'  '-oval'  '--opt'  '--opt=456'
+end
+
 # Check that the argparse's are properly wrapped in begin blocks
 set -l