From f780b01ac9ed81dcb8839191c9dd3ed59b491e68 Mon Sep 17 00:00:00 2001 From: Isaac Oscar Gariano Date: Sun, 3 Aug 2025 10:07:08 +1000 Subject: [PATCH] Added way to tell argparse to delete an option from $argv_opts. The intention is that if you want to parse some of your options verbatim to another command, but you want to modfy other options (e.g. change their value, convert them to other options, or delete them entirely), you mark the options you want to modify with an &, and argparse will not add them to argv_opts. You can then call the other command with argv_opts together with any new/modified options, ensuring that the other command doesn't set the pre-modified options. As with other known options, & options will be removed from $argv, and have their $_flag_ variables set. The `&` goes at the end of the option spec, or if the option spec contains a validation script, immediately before the `!`. There is also now a -d/--delete flag to fish_opt that will generate such an option spec. See the changes in doc_src/cmds/argparse.rst for more details and an example use case. --- CHANGELOG.rst | 1 + doc_src/cmds/argparse.rst | 25 ++++---- doc_src/cmds/fish_opt.rst | 6 +- po/de.po | 3 + po/en.po | 3 + po/fr.po | 3 + po/pl.po | 3 + po/pt_BR.po | 3 + po/sv.po | 3 + po/zh_CN.po | 3 + share/completions/fish_opt.fish | 1 + share/functions/fish_opt.fish | 6 +- src/builtins/argparse.rs | 102 ++++++++++++++++++++++++++++---- src/wgetopt.rs | 10 ++-- tests/checks/argparse.fish | 56 +++++++++++++++--- 15 files changed, 190 insertions(+), 38 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 55e11d4d5..92f3ba9b9 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -43,6 +43,7 @@ Scripting improvements ---------------------- - The ``psub`` command now allows combining ``--suffix`` with ``--fifo`` (:issue:`11729`). - ``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``. Interactive improvements ------------------------ diff --git a/doc_src/cmds/argparse.rst b/doc_src/cmds/argparse.rst index becc7a675..9734da0c9 100644 --- a/doc_src/cmds/argparse.rst +++ b/doc_src/cmds/argparse.rst @@ -112,6 +112,8 @@ Each option specification consists of: - **=+** if it requires a value and each instance of the flag is saved. +- 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. + - Optionally a ``!`` followed by fish script to validate the value. Typically this will be a function to run. If the exit status is zero the value for the flag is valid. If non-zero the value is invalid. Any error messages should be written to stdout (not stderr). See the section on :ref:`Flag Value Validation ` for more information. See the :doc:`fish_opt ` command for a friendlier but more verbose way to create option specifications. @@ -199,6 +201,8 @@ Some *OPTION_SPEC* examples: - ``help`` means that only ``--help`` is valid. The flag is a boolean and can be used more than once. If it is used then ``_flag_help`` will be set as above. Also ``h-help`` (with an arbitrary short letter) for backwards compatibility. +- ``help&`` is similar (it will *remove* ``--help`` from ``$argv``), the difference is that ``--help``` will *not* placed in ``$argv_opts``. + - ``longonly=`` is a flag ``--longonly`` that requires an option, there is no short flag or even short flag variable. - ``n/name=`` means that both ``-n`` and ``--name`` are valid. It requires a value and can be used at most once. If the flag is seen then ``_flag_n`` and ``_flag_name`` will be set with the single mandatory value associated with the flag. @@ -264,22 +268,20 @@ An example of using ``$argv_opts`` to forward known options to another command, 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 - argparse --ignore-unknown $opt_spec -- $argv || return - set -l opts $argv_opts # Save it so it isn't overridden by the next argparse call - # --qwords is a new option, but --bytes is an existing one which we will modify below - argparse qwords= c/bytes= -- $argv || return + set -a opt_spec "qwords=&" "c/bytes=&" + argparse $opt_spec -- $argv || return if set -q _flag_qwords # --qwords allows specifying the size in multiples of 8 bytes - set -a opts --bytes=(math -- $_flag_qwords \* 8 || return) + set -a argv_opts --bytes=(math -- $_flag_qwords \* 8 || return) else if set -q _flag_bytes # Allows using a 'q' suffix, e.g. --bytes=4q to mean 4*8 bytes. if string match -qr 'q$' -- $_flag_bytes - set -a opts --bytes=(math -- (string replace -r 'q$' '*8' -- $_flag_bytes) || return) + set -a argv_opts --bytes=(math -- (string replace -r 'q$' '*8' -- $_flag_bytes) || return) else # Keep the users setting - set -a opts --bytes=$_flag_bytes + set -a argv_opts --bytes=$_flag_bytes end end @@ -290,16 +292,13 @@ An example of using ``$argv_opts`` to forward known options to another command, end # Call the real head with our modified options and arguments. - head $opts -- $argv + head $argv_opts -- $argv end -The first argparse call above saves all the options we do *not* want to process in ``$argv_opts``, which we then copy in to ``$opts``. -The second `argparse` call then parses the options we *do* want to process. The new -value of ``$argv_opts`` is ignored, and instead the ``$_flag_OPTION`` variables are used to transform each of these additional options and -add them back to ``$opts``. +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 first ``argparse`` call is *needed* for the code that inspects ``$argv`` to work, as it checks for the absence of any *non*-option arguments (i.e. no file was specified). We'd similarly need the first call if we wanted to modify the given filenames. +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. Limitations ----------- diff --git a/doc_src/cmds/fish_opt.rst b/doc_src/cmds/fish_opt.rst index d83589bea..780e3f866 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] [-or] [--multiple-vals] [--long-only] + fish_opt -s ALPHANUM [-l LONG-NAME] [-ord] [--multiple-vals] [--long-only] fish_opt --help Description @@ -36,6 +36,10 @@ The following ``argparse`` options are available: **--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. +**-d** or **--delete** + The option and any values will be deleted from the ``$argv_opts`` variables set by ``argparse`` + (as with other options, it will also be deleted from ``$argv``). + **-h** or **--help** Displays help about using this command. diff --git a/po/de.po b/po/de.po index e9235b796..a857bdc2d 100644 --- a/po/de.po +++ b/po/de.po @@ -13617,6 +13617,9 @@ msgstr "" msgid "Delete only those tarballs which aren't present in a snapshot" msgstr "" +msgid "Delete option from argv_opts" +msgstr "" + msgid "Delete previous settime entry" msgstr "" diff --git a/po/en.po b/po/en.po index b7a9c3c0d..55979b817 100644 --- a/po/en.po +++ b/po/en.po @@ -13613,6 +13613,9 @@ msgstr "" msgid "Delete only those tarballs which aren't present in a snapshot" msgstr "" +msgid "Delete option from argv_opts" +msgstr "" + msgid "Delete previous settime entry" msgstr "" diff --git a/po/fr.po b/po/fr.po index ee121ee30..74e1c7e13 100644 --- a/po/fr.po +++ b/po/fr.po @@ -13714,6 +13714,9 @@ msgstr "" msgid "Delete only those tarballs which aren't present in a snapshot" msgstr "" +msgid "Delete option from argv_opts" +msgstr "" + msgid "Delete previous settime entry" msgstr "" diff --git a/po/pl.po b/po/pl.po index 9b010a881..3286f0f95 100644 --- a/po/pl.po +++ b/po/pl.po @@ -13609,6 +13609,9 @@ msgstr "" msgid "Delete only those tarballs which aren't present in a snapshot" msgstr "" +msgid "Delete option from argv_opts" +msgstr "" + msgid "Delete previous settime entry" msgstr "" diff --git a/po/pt_BR.po b/po/pt_BR.po index cf5f8feb6..ddc672c90 100644 --- a/po/pt_BR.po +++ b/po/pt_BR.po @@ -13614,6 +13614,9 @@ msgstr "" msgid "Delete only those tarballs which aren't present in a snapshot" msgstr "" +msgid "Delete option from argv_opts" +msgstr "" + msgid "Delete previous settime entry" msgstr "" diff --git a/po/sv.po b/po/sv.po index 15d7f525c..053e0ab3a 100644 --- a/po/sv.po +++ b/po/sv.po @@ -13612,6 +13612,9 @@ msgstr "" msgid "Delete only those tarballs which aren't present in a snapshot" msgstr "" +msgid "Delete option from argv_opts" +msgstr "" + msgid "Delete previous settime entry" msgstr "" diff --git a/po/zh_CN.po b/po/zh_CN.po index c7fbfadc2..4466dcb16 100644 --- a/po/zh_CN.po +++ b/po/zh_CN.po @@ -13612,6 +13612,9 @@ msgstr "只删除 libvirt 元数据, 留下快照内容" msgid "Delete only those tarballs which aren't present in a snapshot" msgstr "" +msgid "Delete option from argv_opts" +msgstr "" + msgid "Delete previous settime entry" msgstr "删除上一个设定时间项" diff --git a/share/completions/fish_opt.fish b/share/completions/fish_opt.fish index e2d317ad8..299cd1421 100644 --- a/share/completions/fish_opt.fish +++ b/share/completions/fish_opt.fish @@ -10,3 +10,4 @@ complete --command fish_opt --long-option longonly --description 'Use only long 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 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 902978b6a..1a07e677c 100644 --- a/share/functions/fish_opt.fish +++ b/share/functions/fish_opt.fish @@ -11,7 +11,7 @@ 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=' o/optional-val r/required-val + 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 or return @@ -43,5 +43,9 @@ function fish_opt -d 'Produce an option specification suitable for use with `arg and set opt_spec "$opt_spec=?" end + if set -q _flag_delete + set opt_spec "$opt_spec&" + end + echo $opt_spec end diff --git a/src/builtins/argparse.rs b/src/builtins/argparse.rs index cb956eed6..6b74ba935 100644 --- a/src/builtins/argparse.rs +++ b/src/builtins/argparse.rs @@ -34,6 +34,7 @@ struct OptionSpec<'args> { validation_command: &'args wstr, vals: Vec, short_flag_valid: bool, + delete: bool, num_allowed: ArgCardinality, num_seen: isize, } @@ -59,7 +60,7 @@ struct ArgParseCmdOpts<'args> { name: WString, raw_exclusive_flags: Vec<&'args wstr>, args: Vec<&'args wstr>, - args_opts: Vec<&'args wstr>, + args_opts: Vec>, options: HashMap>, long_to_short_flag: HashMap, exclusive_flag_sets: Vec>, @@ -205,7 +206,11 @@ fn parse_flag_modifiers<'args>( ) -> bool { let mut s = *opt_spec_str; - if opt_spec.short_flag == opts.implicit_int_flag && !s.is_empty() && s.char_at(0) != '!' { + if opt_spec.short_flag == opts.implicit_int_flag + && !s.is_empty() + && s.char_at(0) != '!' + && s.char_at(0) != '&' + { streams.err.append(wgettext_fmt!( "%ls: Implicit int short flag '%lc' does not allow modifiers like '%lc'\n", opts.name, @@ -227,6 +232,11 @@ fn parse_flag_modifiers<'args>( } } + if s.char_at(0) == '&' { + opt_spec.delete = true; + s = s.slice_from(1); + } + if s.char_at(0) == '!' { s = s.slice_from(1); opt_spec.validation_command = s; @@ -333,7 +343,7 @@ fn parse_option_spec_sep<'args>( opt_spec.num_allowed = ArgCardinality::Once; i += 1; // the struct is initialized assuming short_flag_valid should be true } - '!' | '?' | '=' => { + '!' | '?' | '=' | '&' => { // Try to parse any other flag modifiers // parse_flag_modifiers assumes opt_spec_str starts where it should, not one earlier s = s.slice_from(i); @@ -680,6 +690,10 @@ fn validate_and_store_implicit_int<'args>( streams: &mut IoStreams, ) -> BuiltinResult { let opt_spec = opts.options.get_mut(&opts.implicit_int_flag).unwrap(); + if opt_spec.delete { + // No need to call delete_flag, as w.argv_opts.last will be of form - + w.argv_opts.pop(); + } validate_arg(parser, &opts.name, opt_spec, is_long_flag, val, streams)?; // It's a valid integer so store it and return success. @@ -691,21 +705,85 @@ fn validate_and_store_implicit_int<'args>( Ok(SUCCESS) } +/// Delete the most recently matched flag, is_long_flag should be true if the flag was given with a +/// long flag name (even if it was given with a single - and not two). +fn delete_flag<'args>(w: &mut WGetopter<'_, 'args, '_>, is_long_flag: bool) { + // Does the option have a value that was a seperate argument to the option name itself? (e.g. + // w.argv_opts ends in -- or -... ) + let separate_value = w + .woptarg + .zip(w.argv_opts.last()) + .is_some_and(|(value, last)| value == *last); + if separate_value { + // Remove the value + w.argv_opts.pop(); + // There can't be any short options between the flag and its value + assert!(w.remaining_text.is_empty()); + } + + // Remove the last option (we may have to put part of it back if it had other short options in + // it) + let opt_arg = w.argv_opts.pop().unwrap(); + assert!(opt_arg.starts_with("-")); + + if is_long_flag { + // opt_arg will be of form --, except there may only be one -, and + // may be abbreviated + assert!(w.remaining_text.is_empty()); + // There will be no short options, so we're done + return; + } + + // Otherwise, it's a short option so opt_arg will be of form: + // -...... + // -..., or + // -... + let more_opts = w.remaining_text; + let value = if separate_value { + L!("") + } else { + w.woptarg.unwrap_or_default() + }; + assert!(more_opts.is_empty() || value.is_empty()); // Both can't be present + assert!(opt_arg.ends_with(more_opts)); + assert!(opt_arg.ends_with(value)); + + // The length of ... (the -2 is for the '-' and ) + let previous_opts = opt_arg.len() - (more_opts.len() + value.len()) - 2; + + if previous_opts == 0 && more_opts.is_empty() { + // opt_arg will be of form - or - + // (i.e. there are no other options that we need to add back to w.argv_opts) + return; + } + + // Set opt_arg_without to to be opt_arg minus (i.e. + // -...). (the +1 is to skip over the leading '-') + let opt_arg_without = opt_arg[..previous_opts + 1].to_owned() + more_opts; + assert!(opt_arg.len() > 1); // There should be at least one short opt + + // Put the version without back + w.argv_opts.push(opt_arg_without.into()); +} + fn handle_flag<'args>( parser: &Parser, opts: &mut ArgParseCmdOpts<'args>, opt: char, is_long_flag: bool, - woptarg: Option<&'args wstr>, + w: &mut WGetopter<'_, 'args, '_>, streams: &mut IoStreams, ) -> BuiltinResult { let opt_spec = opts.options.get_mut(&opt).unwrap(); + if opt_spec.delete { + delete_flag(w, is_long_flag); + } opt_spec.num_seen += 1; if opt_spec.num_allowed == ArgCardinality::None { // It's a boolean flag. Save the flag we saw since it might be useful to know if the // short or long flag was given. - assert!(woptarg.is_none()); + assert!(w.woptarg.is_none()); let s = if is_long_flag { WString::from("--") + opt_spec.long_flag } else { @@ -715,7 +793,7 @@ fn handle_flag<'args>( return Ok(SUCCESS); } - if let Some(woptarg) = woptarg { + if let Some(woptarg) = w.woptarg { validate_arg(parser, &opts.name, opt_spec, is_long_flag, woptarg, streams)?; } @@ -725,12 +803,12 @@ fn handle_flag<'args>( // `opt_spec->num_allowed == 1` and thus return ':' so that we don't take this branch if // the mandatory arg is missing. opt_spec.vals.clear(); - if let Some(arg) = woptarg { + if let Some(arg) = w.woptarg { opt_spec.vals.push(arg.into()); } } _ => { - opt_spec.vals.push(woptarg.unwrap().into()); + opt_spec.vals.push(w.woptarg.unwrap().into()); } } @@ -814,7 +892,7 @@ fn argparse_parse_flags<'args>( continue; } // It's a recognized flag. - _ => handle_flag(parser, opts, opt, is_long_flag, w.woptarg, streams), + _ => handle_flag(parser, opts, opt, is_long_flag, &mut w, streams), }; retval?; } @@ -878,7 +956,7 @@ fn check_min_max_args_constraints( } /// Put the result of parsing the supplied args into the caller environment as local vars. -fn set_argparse_result_vars(vars: &EnvStack, opts: &ArgParseCmdOpts) { +fn set_argparse_result_vars(vars: &EnvStack, opts: ArgParseCmdOpts) { for opt_spec in opts.options.values() { if opt_spec.num_seen == 0 { continue; @@ -904,7 +982,7 @@ fn set_argparse_result_vars(vars: &EnvStack, opts: &ArgParseCmdOpts) { let args = opts.args.iter().map(|&s| s.to_owned()).collect(); vars.set(L!("argv"), EnvMode::LOCAL, args); - let args_opts = opts.args_opts.iter().map(|&s| s.to_owned()).collect(); + let args_opts = opts.args_opts.into_iter().map(|s| s.into_owned()).collect(); vars.set(L!("argv_opts"), EnvMode::LOCAL, args_opts); } @@ -953,7 +1031,7 @@ pub fn argparse(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> check_min_max_args_constraints(&opts, streams)?; - set_argparse_result_vars(parser.vars(), &opts); + set_argparse_result_vars(parser.vars(), opts); Ok(SUCCESS) } diff --git a/src/wgetopt.rs b/src/wgetopt.rs index 15ab0e8e4..2abc8485f 100644 --- a/src/wgetopt.rs +++ b/src/wgetopt.rs @@ -107,6 +107,8 @@ enum NextArgv { FoundOption, } +use std::borrow::Cow; + pub struct WGetopter<'opts, 'args, 'argarray> { /// List of arguments. Will not be resized, but can be modified. pub argv: &'argarray mut [&'args wstr], @@ -141,7 +143,7 @@ pub struct WGetopter<'opts, 'args, 'argarray> { initialized: bool, /// This will be populated with the elements of the original args that were interpreted /// as options and arguments to options - pub argv_opts: Vec<&'args wstr>, + pub argv_opts: Vec>, } impl<'opts, 'args, 'argarray> WGetopter<'opts, 'args, 'argarray> { @@ -307,7 +309,7 @@ fn next_argv(&mut self) -> NextArgv { } let opt = self.argv[self.wopt_index]; - self.argv_opts.push(opt); + self.argv_opts.push(Cow::Borrowed(opt)); // We've found an option, so we need to skip the initial punctuation. let skip = if !self.longopts.is_empty() && opt.char_at(1) == '-' { @@ -373,7 +375,7 @@ fn handle_short_opt(&mut self) -> char { } else { // Consume the next element. let val = self.argv[self.wopt_index]; - self.argv_opts.push(val); + self.argv_opts.push(Cow::Borrowed(val)); self.woptarg = Some(val); self.wopt_index += 1; } @@ -403,7 +405,7 @@ fn update_long_opt( } else if opt_found.arg_type == ArgType::RequiredArgument { if self.wopt_index < self.argv.len() { let val = self.argv[self.wopt_index]; - self.argv_opts.push(val); + self.argv_opts.push(Cow::Borrowed(val)); self.woptarg = Some(val); self.wopt_index += 1; } else { diff --git a/tests/checks/argparse.fish b/tests/checks/argparse.fish index f4d800e06..d1c96bb84 100644 --- a/tests/checks/argparse.fish +++ b/tests/checks/argparse.fish @@ -473,10 +473,10 @@ fish_opt --short h --long help -r --long-only or echo unexpected status $status #CHECK: h-help= -# Optional val, short and long valid -fish_opt --short h -l help --optional-val +# Optional val, short and long valid, and delete +fish_opt --short h -l help --optional-val --delete or echo unexpected status $status -#CHECK: h/help=? +#CHECK: h/help=?& # Optional val, short and long but the short var cannot be used fish_opt --short h -l help --optional-val --long-only @@ -493,13 +493,13 @@ fish_opt --short h -l help --multiple-vals --long-only or echo unexpected status $status #CHECK: h-help=+ -# Repeated val, short only +# Repeated val, short only, and deleted fish_opt -s h --multiple-vals or echo unexpected status $status -fish_opt -s h --multiple-vals --long-only +#CHECK: h=+ +fish_opt -s h --multiple-vals --long-only -d or echo unexpected status $status -#CHECK: h=+ -#CHECK: h=+ +#CHECK: h=+& function wrongargparse argparse -foo -- banana @@ -528,5 +528,47 @@ begin #CHECK: No flag I end +# Tests for --delete +begin + argparse 'a/long&' -- before -a inbetween --long after + set -l + # CHECK: _flag_a '-a' '--long' + # CHECK: _flag_long '-a' '--long' + # CHECK: argv 'before' 'inbetween' 'after' + # CHECK: argv_opts +end + +begin + argparse 'a/long=+&' '#int&' -- -123 before -a -a inbetween -astuck ater -long=long + set -l + # CHECK: _flag_a '-a' 'stuck' 'long' + # CHECK: _flag_int 123 + # CHECK: _flag_long '-a' 'stuck' 'long' + # CHECK: argv 'before' 'inbetween' 'ater' + # CHECK: argv_opts +end + + +begin + argparse 'd=?&' a b -- -d -d3 -ad -bd345 + set -l + # CHECK: _flag_a -a + # CHECK: _flag_b -b + # CHECK: _flag_d 345 + # CHECK: argv + # CHECK: argv_opts '-a' '-b' +end + +begin + argparse 'd&' a b 'v=' -- 0 -adbv124 1 -abdv125 2 -dabv124 3 -vd3 + set -l + # CHECK: _flag_a '-a' '-a' '-a' + # CHECK: _flag_b '-b' '-b' '-b' + # CHECK: _flag_d '-d' '-d' '-d' + # CHECK: _flag_v d3 + # CHECK: argv '0' '1' '2' '3' + # CHECK: argv_opts '-abv124' '-abv125' '-abv124' '-vd3' +end + # Check that the argparse's are properly wrapped in begin blocks set -l