Fixed some double-dash handling logic added unit tests

2 files changed, 72 insertions, 56 deletions

diff --git a/cmd2/argparse_completer.py b/cmd2/argparse_completer.py
index 27e7df25..d7ffc180 100644
--- a/cmd2/argparse_completer.py
+++ b/cmd2/argparse_completer.py
@@ -288,19 +288,31 @@ class AutoCompleter(object):
             if token_index >= len(tokens) - 1:
                 is_last_token = True
 
-            # If a remainder action is found, force all future tokens to go to that
+            # If we're in a positional REMAINDER arg, force all future tokens to go to that
             if pos_arg_state is not None and pos_arg_state.is_remainder:
                 consume_positional_argument()
                 continue
+
+            # If we're in a flag REMAINDER arg, force all future tokens to go to that until a double dash is hit
             elif flag_arg_state is not None and flag_arg_state.is_remainder:
                 skip_remaining_flags = True
                 if token == '--':
-                    # End this flag and don't allow any more flags
                     flag_arg_state = None
                 else:
                     consume_flag_argument()
                 continue
 
+            # Handle '--' which tells argparse all remaining arguments are non-flags
+            elif token == '--' and not skip_remaining_flags:
+                if is_last_token:
+                    # Exit loop and see if -- can be completed into a flag
+                    break
+                else:
+                    # End the current flag
+                    flag_arg_state = None
+                    skip_remaining_flags = True
+                    continue
+
             current_is_positional = False
 
             # Are we consuming flag arguments?
@@ -319,16 +331,8 @@ class AutoCompleter(object):
                             self._positional_actions[next_pos_arg_index].nargs == argparse.REMAINDER:
                         skip_remaining_flags = True
 
-                # Handle '--' which tells argparse all remaining arguments are non-flags
-                if token == '--' and not skip_remaining_flags:
-                    if is_last_token:
-                        # Exit loop and see if -- can be completed into a flag
-                        break
-                    else:
-                        skip_remaining_flags = True
-
                 # At this point we're no longer consuming flag arguments. Is the current argument a potential flag?
-                elif is_potential_flag(token, self._parser) and not skip_remaining_flags:
+                if is_potential_flag(token, self._parser) and not skip_remaining_flags:
                     # Reset flag arg state but not positional tracking because flags can be
                     # interspersed anywhere between positionals
                     flag_arg_state = None
diff --git a/tests/test_argparse_completer.py b/tests/test_argparse_completer.py
index 404ba10b..cf4ac7b3 100644
--- a/tests/test_argparse_completer.py
+++ b/tests/test_argparse_completer.py
@@ -19,6 +19,11 @@ static_choices_list = ['static', 'choices', 'stop', 'here']
 choices_from_function = ['choices', 'function', 'chatty', 'smith']
 choices_from_method = ['choices', 'method', 'most', 'improved']
 
+set_value_choices = ['set', 'value', 'choices']
+one_or_more_choices = ['one', 'or', 'more', 'choices']
+optional_choices = ['optional', 'choices']
+remainder_choices = ['remainder', 'choices']
+
 completions_from_function = ['completions', 'function', 'fairly', 'complete']
 completions_from_method = ['completions', 'method', 'missed', 'spot']
 
@@ -185,11 +190,13 @@ class AutoCompleteTester(cmd2.Cmd):
 
     # Flag args for nargs command
     nargs_parser.add_argument("--set_value", help="a flag with a set value for nargs", nargs=2,
-                              choices=static_choices_list)
+                              choices=set_value_choices)
     nargs_parser.add_argument("--one_or_more", help="a flag wanting one or more args", nargs=argparse.ONE_OR_MORE,
-                              choices=static_choices_list)
+                              choices=one_or_more_choices)
+    nargs_parser.add_argument("--optional", help="a flag with an optional value", nargs=argparse.OPTIONAL,
+                              choices=optional_choices)
     nargs_parser.add_argument("--remainder", help="a flag wanting remaining", nargs=argparse.REMAINDER,
-                              choices=static_choices_list)
+                              choices=remainder_choices)
 
     @with_argparser(nargs_parser)
     def do_nargs(self, args: argparse.Namespace) -> None:
@@ -218,48 +225,6 @@ def ac_app():
     return app
 
 
-@pytest.mark.parametrize('args, completions', [
-    # Flag with nargs = 2
-    ('--set_value', static_choices_list),
-    ('--set_value static', ['choices', 'stop', 'here']),
-    ('--set_value static choices', []),
-
-    # Using the flag again will reset the choices available
-    ('--set_value static choices --set_value', static_choices_list),
-
-    # Flag with nargs = ONE_OR_MORE
-    ('--one_or_more', static_choices_list),
-    ('--one_or_more static', ['choices', 'stop', 'here']),
-    ('--one_or_more static choices', ['stop', 'here']),
-
-    # No more flags after a double dash
-    ('-- --one_or_more static choices', []),
-
-    # Flag with nargs = REMAINDER
-    ('--remainder', static_choices_list),
-    ('--remainder static ', ['choices', 'stop', 'here']),
-
-    # No more flags can appear after a REMAINDER flag)
-    ('--remainder static --set_value', ['choices', 'stop', 'here']),
-
-    # Double dash ends a remainder flag
-    ('--remainder static --', [])
-])
-def test_autcomp_nargs(ac_app, args, completions):
-    text = ''
-    line = 'nargs {} {}'.format(args, text)
-    endidx = len(line)
-    begidx = endidx - len(text)
-
-    first_match = complete_tester(text, line, begidx, endidx, ac_app)
-    if completions:
-        assert first_match is not None
-    else:
-        assert first_match is None
-
-    assert ac_app.completion_matches == sorted(completions, key=ac_app.matches_sort_key)
-
-
 @pytest.mark.parametrize('command', [
     'music',
     'music create',
@@ -465,6 +430,53 @@ def test_completion_items(ac_app, num_aliases, show_description):
     assert ('help' in ac_app.display_matches[0]) == show_description
 
 
+@pytest.mark.parametrize('args, completions', [
+    # Flag with nargs = 2
+    ('--set_value', set_value_choices),
+    ('--set_value set', ['value', 'choices']),
+    ('--set_value set value choices', []),
+
+    # Another flag can't start until all expected args are filled out
+    ('--set_value --one_or_more', set_value_choices),
+
+    # Using the flag again will reset the choices available
+    ('--set_value set value --set_value', set_value_choices),
+
+    # Flag with nargs = ONE_OR_MORE
+    ('--one_or_more', one_or_more_choices),
+    ('--one_or_more one', ['or', 'more', 'choices']),
+
+    # Flag with nargs = REMAINDER
+    ('--remainder', remainder_choices),
+    ('--remainder remainder ', ['choices ']),
+
+    # No more flags can appear after a REMAINDER flag)
+    ('--remainder choices --set_value', ['remainder ']),
+
+    # Double dash ends the current flag (even if all expected args aren't entered)
+    ('--set_value --', []),
+
+    # Double dash ends a REMAINDER flag
+    ('--remainder remainder --', []),
+
+    # No more flags after a double dash
+    ('-- --one_or_more ', []),
+])
+def test_autcomp_nargs(ac_app, args, completions):
+    text = ''
+    line = 'nargs {} {}'.format(args, text)
+    endidx = len(line)
+    begidx = endidx - len(text)
+
+    first_match = complete_tester(text, line, begidx, endidx, ac_app)
+    if completions:
+        assert first_match is not None
+    else:
+        assert first_match is None
+
+    assert ac_app.completion_matches == sorted(completions, key=ac_app.matches_sort_key)
+
+
 def test_completion_items_default_header(ac_app):
     from cmd2.argparse_completer import DEFAULT_DESCRIPTIVE_HEADER