From 4db61ee117c954a441213bfdb47005fc68d257e6 Mon Sep 17 00:00:00 2001 From: Isaac Oscar Gariano Date: Tue, 19 Aug 2025 13:25:26 +1000 Subject: [PATCH] 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=[]: 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 | 1 + doc_src/cmds/argparse.rst | 10 ++++++--- doc_src/cmds/fish_opt.rst | 6 +++--- share/completions/fish_opt.fish | 2 +- share/functions/fish_opt.fish | 10 +++++---- src/builtins/argparse.rs | 10 ++++++++- tests/checks/argparse.fish | 38 +++++++++++++++++++++++++++------ 7 files changed, 58 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index a3c5516ad..d892111c0 100644 --- a/CHANGELOG.rst +++ b/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 ------------------------ diff --git a/doc_src/cmds/argparse.rst b/doc_src/cmds/argparse.rst index 08be7500c..d923e37af 100644 --- a/doc_src/cmds/argparse.rst +++ b/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. diff --git a/doc_src/cmds/fish_opt.rst b/doc_src/cmds/fish_opt.rst index 780e3f866..97d11965c 100644 --- a/doc_src/cmds/fish_opt.rst +++ b/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`` diff --git a/share/completions/fish_opt.fish b/share/completions/fish_opt.fish index 299cd1421..6a8e14d0a 100644 --- a/share/completions/fish_opt.fish +++ b/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' diff --git a/share/functions/fish_opt.fish b/share/functions/fish_opt.fish index 1a07e677c..21e99730a 100644 --- a/share/functions/fish_opt.fish +++ b/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=" diff --git a/src/builtins/argparse.rs b/src/builtins/argparse.rs index 870b7b325..796812449 100644 --- a/src/builtins/argparse.rs +++ b/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 diff --git a/tests/checks/argparse.fish b/tests/checks/argparse.fish index f918fae19..357a0cfad 100644 --- a/tests/checks/argparse.fish +++ b/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