diff --git a/CHANGELOG.rst b/CHANGELOG.rst index d1d5313e8..a3c5516ad 100644 --- a/CHANGELOG.rst +++ b/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 ------------------------ diff --git a/doc_src/cmds/argparse.rst b/doc_src/cmds/argparse.rst index d6e234fe4..08be7500c 100644 --- a/doc_src/cmds/argparse.rst +++ b/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`` 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). +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``, or ``--opt=``). + +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``. diff --git a/po/de.po b/po/de.po index 73c23f162..9170834b3 100644 --- a/po/de.po +++ b/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" diff --git a/po/en.po b/po/en.po index 04529cc66..a4c1f0b42 100644 --- a/po/en.po +++ b/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 "" diff --git a/po/fr.po b/po/fr.po index 53ecb4a07..cb66cfecc 100644 --- a/po/fr.po +++ b/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" diff --git a/po/pl.po b/po/pl.po index 1ff91e2b3..9b50bbe2b 100644 --- a/po/pl.po +++ b/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 "" diff --git a/po/pt_BR.po b/po/pt_BR.po index e67b25df8..71834756f 100644 --- a/po/pt_BR.po +++ b/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 "" diff --git a/po/sv.po b/po/sv.po index 16adf9ac6..8d108af6c 100644 --- a/po/sv.po +++ b/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 "" diff --git a/po/zh_CN.po b/po/zh_CN.po index 66ffad875..394b1bc3c 100644 --- a/po/zh_CN.po +++ b/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" diff --git a/share/completions/argparse.fish b/share/completions/argparse.fish index fd419fc23..622f2e3c2 100644 --- a/share/completions/argparse.fish +++ b/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' diff --git a/src/builtins/argparse.rs b/src/builtins/argparse.rs index 105ab3f73..870b7b325 100644 --- a/src/builtins/argparse.rs +++ b/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 --=, so extract out + 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. --=) + 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; diff --git a/tests/checks/argparse.fish b/tests/checks/argparse.fish index 740ae38be..f918fae19 100644 --- a/tests/checks/argparse.fish +++ b/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