Added an -S/--strict-longopts option to argparse.

This flag disables a very surprising and confusing feature I found in the code
of wgetopt.rs: the ability to abbreviate the names of long options and the
ability to parse long options with a single "-". This commit addresses #7341,
but unlike pull request #11220, it does so in a backwards compatible way: one
must use the new -S/--strict-longotps flag to disable the old legacy behaviour.

Unlike pull request #11220 however, this flag only applies to ``argparse``,
and not to any builtins used by fish.

Note that forcing the flag -S/--strict-longotps on (i.e. in  src/wgetopt.rs,
replacing both uses of `self.strict_long_opts` with `true`), does not cause any
of the current test cases to fail. However, third-party fish scripts may be
depending on the current behaviour.
This commit is contained in:
Isaac Oscar Gariano
2025-08-03 10:07:08 +10:00
parent 70dca1e136
commit 24eeed65a2
6 changed files with 65 additions and 5 deletions

View File

@@ -46,6 +46,7 @@ Scripting improvements
- ``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``.
- ``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).
Interactive improvements
------------------------

View File

@@ -50,6 +50,21 @@ The following ``argparse`` options are available. They must appear before all *O
**-i** or **--ignore-unknown**
Deprecated. This is like **--move-unknown**, except that unknown options and their arguments are kept in ``$argv`` and not moved to ``$argv_opts``. Unlike **--move-unknown**, this option makes it impossible to distinguish between an unknown option and non-option argument that starts with a ``-`` (since any ``--`` seperator in ``$argv`` will be removed).
**-S** or **--strict-longopts**
This makes the parsing of long options more strict. In particular, *without* this flag, if ``long`` is a known long option flag, ``--long`` and ``--long=<value>`` can be abbreviated as:
- ``-long`` and ``-long=<value>``, but *only* if there is no short flag ``l``.
- ``--lo`` and ``--lo=<value>``, but *only* if there is no other long flag that starts with ``lo``. Similarly with any other non-empty prefix of ``long``.
- ``-lo`` and ``-lo=<value>`` (i.e. combining the above two).
With the ``--strict-longopts`` flag, the above three are parse errors: one must use the syntax ``--long`` or ``--long=<value>`` to use a long option called ``long``.
This flag has no effect on the parsing of unknown options (which are parsed as if this flag is on).
This option may be on all the time in the future, so do not rely on the behaviour without it.
**-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.

View File

@@ -72,5 +72,7 @@ complete --command argparse --short-option i --long-option ignore-unknown \
complete --command argparse --short-option u --long-option move-unknown \
-n '! __fish_seen_argument --short i --long ignore-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 s --long-option stop-nonopt \
--description 'Exit on subcommand'

View File

@@ -60,6 +60,7 @@ enum UnknownHandling {
#[derive(Default)]
struct ArgParseCmdOpts<'args> {
unknown_handling: UnknownHandling,
strict_long_opts: bool,
print_help: bool,
stop_nonopt: bool,
min_args: usize,
@@ -83,13 +84,14 @@ fn new() -> Self {
}
}
const SHORT_OPTIONS: &wstr = L!("+hn:siux:N:X:");
const SHORT_OPTIONS: &wstr = L!("+hn:siux: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!("name"), ArgType::RequiredArgument, 'n'),
wopt(L!("exclusive"), ArgType::RequiredArgument, 'x'),
wopt(L!("strict-longopts"), ArgType::NoArgument, 'S'),
wopt(L!("help"), ArgType::NoArgument, 'h'),
wopt(L!("min-args"), ArgType::RequiredArgument, 'N'),
wopt(L!("max-args"), ArgType::RequiredArgument, 'X'),
@@ -532,6 +534,7 @@ fn parse_cmd_opts<'args>(
UnknownHandling::Move
}
}
'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.
'x' => opts.raw_exclusive_flags.push(w.woptarg.unwrap()),
@@ -873,6 +876,7 @@ fn argparse_parse_flags<'args>(
populate_option_strings(opts, &mut short_options, &mut long_options);
let mut w = WGetopter::new(&short_options, &long_options, 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 {

View File

@@ -139,6 +139,9 @@ pub struct WGetopter<'opts, 'args, 'argarray> {
pub last_nonopt: usize,
/// Prevents redundant initialization.
initialized: bool,
// Makes parsing long options more strict (long options must have their full name specified,
// and cannot be given with a single hyphen).
pub strict_long_opts: 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<Cow<'args, wstr>>,
@@ -162,6 +165,7 @@ pub fn new(
first_nonopt: 0,
last_nonopt: 0,
initialized: false,
strict_long_opts: false,
argv_opts: Vec::new(),
}
}
@@ -438,6 +442,9 @@ fn find_matching_long_opt(&self, name_end: usize) -> LongOptMatch<'opts> {
index = i;
exact = true;
break;
} else if self.strict_long_opts {
// It didn't match exactly, continue
continue;
} else if opt.is_none() {
// The option begins with the matching text, but is not
// exactly equal.
@@ -531,9 +538,9 @@ fn wgetopt_inner(&mut self, longopt_index: &mut usize) -> Option<char> {
}
// We set things up so that `-f` is parsed as a short-named option if there
// is a valid option to match it to, otherwise we parse it as a long-named
// option. We also make sure that `-fu` is *not* parsed as `-f` with
// an arg `u`.
// is a valid option to match it to, otherwise, if self.strict_long_opts is false,
// we parse it as a long-named option. We also make sure that `-fu` is *not* parsed as
// `-f` with an arg `u`.
if !self.longopts.is_empty() && self.wopt_index < self.argv.len() {
let arg = self.argv[self.wopt_index];
@@ -541,7 +548,8 @@ fn wgetopt_inner(&mut self, longopt_index: &mut usize) -> Option<char> {
// matches options like `--foo`
arg.char_at(0) == '-' && arg.char_at(1) == '-'
// matches options like `-f` if `f` is not a valid shortopt.
|| !self.shortopts.as_char_slice().contains(&arg.char_at(1));
|| (!self.strict_long_opts
&& !self.shortopts.as_char_slice().contains(&arg.char_at(1)));
if try_long {
let retval = self.handle_long_opt(longopt_index);

View File

@@ -645,5 +645,35 @@ argparse a/alpha -- --banna=value
argparse a/alpha -- --alpha=value --banna=value
# CHECKERR: argparse: --alpha=value: option does not take an argument
# Check behaviour without -S/--strict-longopts option
begin
argparse long valu=+ -- --lon -long -lon -valu=3 -valu 4 -val=3 -val 4
set -lL
# CHECK: _flag_long '--long' '--long' '--long'
# CHECK: _flag_valu '3' '4' '3' '4'
# CHECK: argv
# CHECK: argv_opts '--lon' '-long' '-lon' '-valu=3' '-valu' '4' '-val=3' '-val' '4'
argparse amb ambig -- -am
# CHECKERR: argparse: -am: unknown option
argparse a ambig -- -ambig
# CHECKERR: argparse: -ambig: unknown option
argparse long -- -long3
# CHECKERR: argparse: -long3: unknown option
end
# Check behaviour with -S/--strict-longopts option
begin
argparse -S long -- --lon
# CHECKERR: argparse: --lon: unknown option
argparse -S long -- -long
# CHECKERR: argparse: -long: unknown option
argparse --strict-longopts long -- -lon
# CHECKERR: argparse: -lon: unknown option
argparse -S value=+ -- -valu=3
# CHECKERR: argparse: -valu=3: unknown option
argparse --strict-longopts value=+ -- -valu 4
# CHECKERR: argparse: -valu: unknown option
end
# Check that the argparse's are properly wrapped in begin blocks
set -l