From 0781473564dd89d13c646659a9d957d46607fcf3 Mon Sep 17 00:00:00 2001
From: Fabian Homborg <FHomborg@gmail.com>
Date: Sat, 15 Jan 2022 12:17:43 +0100
Subject: [PATCH] argparse: Jump to the next option after an unknown one

Previously, when we got an unknown option with --ignore-unknown, we
would increment woptind but still try to read the same contents.

This means in e.g.

```
argparse -i h -- -ooo -h
```

The `-h` would also be skipped as an option, because after the first
`-o` getopt reads the other two `-o` and skips that many options.

This could be handled more extensively in wgetopt, but the simpler fix
is to just skip to the next argv entry once we have an unknown option
- there's nothing more we can do with it anyway!

Additionally, document this and clearly explain that we currently
don't transform the option.

Fixes #8637
---
 doc_src/cmds/argparse.rst  | 15 +++++++++++++++
 src/builtins/argparse.cpp  |  3 +++
 tests/checks/argparse.fish | 11 +++++++++++
 3 files changed, 29 insertions(+)

diff --git a/doc_src/cmds/argparse.rst b/doc_src/cmds/argparse.rst
index dd6e8e674..dff27444c 100644
--- a/doc_src/cmds/argparse.rst
+++ b/doc_src/cmds/argparse.rst
@@ -184,3 +184,18 @@ Some OPTION_SPEC examples:
 After parsing the arguments the ``argv`` variable is set with local scope to any values not already consumed during flag processing. If there are no unbound values the variable is set but ``count $argv`` will be zero.
 
 If an error occurs during argparse processing it will exit with a non-zero status and print error messages to stderr.
+
+Limitations
+-----------
+
+One limitation with **--ignore-unknown** is that, if an unknown option is given in a group with known options, the entire group will be kept in $argv. ``argparse`` will not do any permutations here.
+
+For instance::
+
+  argparse --ignore-unknown h -- -ho
+  echo $_flag_h # is -h, because -h was given
+  echo $argv # is still -ho
+
+This limitation may be lifted in future.
+
+Additionally, it can only parse known options up to the first unknown option in the group - the unknown option could take options, so it isn't clear what any character after an unknown option means.
diff --git a/src/builtins/argparse.cpp b/src/builtins/argparse.cpp
index 53824abc3..9f06d19ac 100644
--- a/src/builtins/argparse.cpp
+++ b/src/builtins/argparse.cpp
@@ -596,6 +596,9 @@ static int argparse_parse_flags(parser_t &parser, argparse_cmd_opts_t &opts,
                 opts.argv.push_back(arg_contents - 1);
                 // Work around weirdness with wgetopt, which crashes if we `continue` here.
                 if (w.woptind == argc) break;
+                // Explain to wgetopt that we want to skip to the next arg,
+                // because we can't handle this opt group.
+                w.nextchar = nullptr;
             }
             if (retval != STATUS_CMD_OK) return retval;
             long_idx = -1;
diff --git a/tests/checks/argparse.fish b/tests/checks/argparse.fish
index a55844b63..9a5885de1 100644
--- a/tests/checks/argparse.fish
+++ b/tests/checks/argparse.fish
@@ -498,3 +498,14 @@ begin
     #CHECKERR: ^
     #CHECKERR: (Type 'help argparse' for related documentation)
 end
+
+begin
+    argparse --ignore-unknown h i -- -hoa -oia
+    echo -- $argv
+    #CHECK: -hoa -oia
+    echo $_flag_h
+    #CHECK: -h
+    set -q _flag_i
+    or echo No flag I
+    #CHECK: No flag I
+end