Merge pull request #573 from python-cmd2/double_dash

Double dash

9 files changed, 123 insertions, 61 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 0042b86b..d79957fd 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -14,6 +14,8 @@
     the argparse object. Also, single-character tokens that happen to be a
     prefix char are not treated as flags by argparse and AutoCompleter now
     matches that behavior.
+    * Fixed bug where AutoCompleter was not distinguishing between a negative number and a flag 
+    * Fixed bug where AutoCompleter did not handle -- the same way argparse does (all args after -- are non-options)  
 * Enhancements
     * Added ``exit_code`` attribute of ``cmd2.Cmd`` class
         * Enables applications to return a non-zero exit code when exiting from ``cmdloop``
diff --git a/cmd2/argparse_completer.py b/cmd2/argparse_completer.py
index 168a555f..77a62b9d 100755..100644
--- a/cmd2/argparse_completer.py
+++ b/cmd2/argparse_completer.py
@@ -209,8 +209,8 @@ def register_custom_actions(parser: argparse.ArgumentParser) -> None:
     parser.register('action', 'append', _AppendRangeAction)
 
 
-def token_resembles_flag(token: str, parser: argparse.ArgumentParser) -> bool:
-    """Determine if a token looks like a flag. Based on argparse._parse_optional()."""
+def is_potential_flag(token: str, parser: argparse.ArgumentParser) -> bool:
+    """Determine if a token looks like a potential flag. Based on argparse._parse_optional()."""
     # if it's an empty string, it was meant to be a positional
     if not token:
         return False
@@ -340,6 +340,10 @@ class AutoCompleter(object):
         # Skip any flags or flag parameter tokens
         next_pos_arg_index = 0
 
+        # This gets set to True when flags will no longer be processed as argparse flags
+        # That can happen when -- is used or an argument with nargs=argparse.REMAINDER is used
+        skip_remaining_flags = False
+
         pos_arg = AutoCompleter._ArgumentState()
         pos_action = None
 
@@ -363,7 +367,7 @@ class AutoCompleter(object):
             """Consuming token as a flag argument"""
             # we're consuming flag arguments
             # if the token does not look like a new flag, then count towards flag arguments
-            if not token_resembles_flag(token, self._parser) and flag_action is not None:
+            if not is_potential_flag(token, self._parser) and flag_action is not None:
                 flag_arg.count += 1
 
                 # does this complete a option item for the flag
@@ -432,8 +436,10 @@ class AutoCompleter(object):
 
         for idx, token in enumerate(tokens):
             is_last_token = idx >= len(tokens) - 1
+
             # Only start at the start token index
             if idx >= self._token_start_index:
+
                 # If a remainder action is found, force all future tokens to go to that
                 if remainder['arg'] is not None:
                     if remainder['action'] == pos_action:
@@ -442,28 +448,38 @@ class AutoCompleter(object):
                     elif remainder['action'] == flag_action:
                         consume_flag_argument()
                         continue
+
                 current_is_positional = False
                 # Are we consuming flag arguments?
                 if not flag_arg.needed:
-                    # Special case when each of the following is true:
-                    #   - We're not in the middle of consuming flag arguments
-                    #   - The current positional argument count has hit the max count
-                    #   - The next positional argument is a REMAINDER argument
-                    # Argparse will now treat all future tokens as arguments to the positional including tokens that
-                    # look like flags so the completer should skip any flag related processing once this happens
-                    skip_flag = False
-                    if (pos_action is not None) and pos_arg.count >= pos_arg.max and \
-                            next_pos_arg_index < len(self._positional_actions) and \
-                            self._positional_actions[next_pos_arg_index].nargs == argparse.REMAINDER:
-                        skip_flag = True
+
+                    if not skip_remaining_flags:
+                        # Special case when each of the following is true:
+                        #   - We're not in the middle of consuming flag arguments
+                        #   - The current positional argument count has hit the max count
+                        #   - The next positional argument is a REMAINDER argument
+                        # Argparse will now treat all future tokens as arguments to the positional including tokens that
+                        # look like flags so the completer should skip any flag related processing once this happens
+                        if (pos_action is not None) and pos_arg.count >= pos_arg.max and \
+                                next_pos_arg_index < len(self._positional_actions) and \
+                                self._positional_actions[next_pos_arg_index].nargs == argparse.REMAINDER:
+                            skip_remaining_flags = True
 
                     # At this point we're no longer consuming flag arguments. Is the current argument a potential flag?
-                    if token_resembles_flag(token, self._parser) and not skip_flag:
+                    if is_potential_flag(token, self._parser) and not skip_remaining_flags:
                         # reset some tracking values
                         flag_arg.reset()
                         # don't reset positional tracking because flags can be interspersed anywhere between positionals
                         flag_action = None
 
+                        if token == '--':
+                            if is_last_token:
+                                # Exit loop and see if -- can be completed into a flag
+                                break
+                            else:
+                                # In argparse, all args after -- are non-flags
+                                skip_remaining_flags = True
+
                         # does the token fully match a known flag?
                         if token in self._flag_to_action:
                             flag_action = self._flag_to_action[token]
@@ -524,22 +540,25 @@ class AutoCompleter(object):
                 else:
                     consume_flag_argument()
 
+                if remainder['arg'] is not None:
+                    skip_remaining_flags = True
+
                 # don't reset this if we're on the last token - this allows completion to occur on the current token
-                if not is_last_token and flag_arg.min is not None:
+                elif not is_last_token and flag_arg.min is not None:
                     flag_arg.needed = flag_arg.count < flag_arg.min
 
         # Here we're done parsing all of the prior arguments. We know what the next argument is.
 
+        completion_results = []
+
         # if we don't have a flag to populate with arguments and the last token starts with
         # a flag prefix then we'll complete the list of flag options
-        completion_results = []
         if not flag_arg.needed and len(tokens[-1]) > 0 and tokens[-1][0] in self._parser.prefix_chars and \
-                remainder['arg'] is None:
+                not skip_remaining_flags:
             return AutoCompleter.basic_complete(text, line, begidx, endidx,
                                                 [flag for flag in self._flags if flag not in matched_flags])
         # we're not at a positional argument, see if we're in a flag argument
         elif not current_is_positional:
-            # current_items = []
             if flag_action is not None:
                 consumed = consumed_arg_values[flag_action.dest]\
                     if flag_action.dest in consumed_arg_values else []
diff --git a/cmd2/cmd2.py b/cmd2/cmd2.py
index c000fb80..02803b06 100644
--- a/cmd2/cmd2.py
+++ b/cmd2/cmd2.py
@@ -3563,8 +3563,8 @@ a..b, a:b, a:, ..b  items by indices (inclusive)
             except AttributeError:
                 # Debugging in Pycharm has issues with setting terminal title
                 pass
-
-            self.terminal_lock.release()
+            finally:
+                self.terminal_lock.release()
 
         else:
             raise RuntimeError("another thread holds terminal_lock")
diff --git a/cmd2/pyscript_bridge.py b/cmd2/pyscript_bridge.py
index 11a2cbb3..3c5c61f2 100644
--- a/cmd2/pyscript_bridge.py
+++ b/cmd2/pyscript_bridge.py
@@ -12,7 +12,7 @@ import functools
 import sys
 from typing import List, Callable, Optional
 
-from .argparse_completer import _RangeAction, token_resembles_flag
+from .argparse_completer import _RangeAction, is_potential_flag
 from .utils import namedtuple_with_defaults, StdSim, quote_string_if_needed
 
 # Python 3.4 require contextlib2 for temporarily redirecting stderr and stdout
@@ -222,10 +222,12 @@ class ArgparseFunctor:
             if action.option_strings:
                 cmd_str[0] += '{} '.format(action.option_strings[0])
 
+            is_remainder_arg = action.dest == self._remainder_arg
+
             if isinstance(value, List) or isinstance(value, tuple):
                 for item in value:
                     item = str(item).strip()
-                    if token_resembles_flag(item, self._parser):
+                    if not is_remainder_arg and is_potential_flag(item, self._parser):
                         raise ValueError('{} appears to be a flag and should be supplied as a keyword argument '
                                          'to the function.'.format(item))
                     item = quote_string_if_needed(item)
@@ -240,7 +242,7 @@ class ArgparseFunctor:
 
             else:
                 value = str(value).strip()
-                if token_resembles_flag(value, self._parser):
+                if not is_remainder_arg and is_potential_flag(value, self._parser):
                     raise ValueError('{} appears to be a flag and should be supplied as a keyword argument '
                                      'to the function.'.format(value))
                 value = quote_string_if_needed(value)
diff --git a/examples/tab_autocompletion.py b/examples/tab_autocompletion.py
index 571b4082..dad9e90d 100755
--- a/examples/tab_autocompletion.py
+++ b/examples/tab_autocompletion.py
@@ -163,7 +163,7 @@ class TabCompleteExample(cmd2.Cmd):
 
     # This variant demonstrates the AutoCompleter working with the orginial argparse.
     # Base argparse is unable to specify narg ranges. Autocompleter will keep expecting additional arguments
-    # for the -d/--duration flag until you specify a new flaw or end the list it with '--'
+    # for the -d/--duration flag until you specify a new flag or end processing of flags with '--'
 
     suggest_parser_orig = argparse.ArgumentParser()
 
diff --git a/tests/test_acargparse.py b/tests/test_acargparse.py
index 617afd4f..b6abc444 100644
--- a/tests/test_acargparse.py
+++ b/tests/test_acargparse.py
@@ -5,7 +5,7 @@ Copyright 2018 Eric Lin <anselor@gmail.com>
 Released under MIT license, see LICENSE file
 """
 import pytest
-from cmd2.argparse_completer import ACArgumentParser, token_resembles_flag
+from cmd2.argparse_completer import ACArgumentParser, is_potential_flag
 
 
 def test_acarg_narg_empty_tuple():
@@ -53,16 +53,16 @@ def test_acarg_narg_tuple_zero_to_one():
     parser.add_argument('tuple', nargs=(0, 1))
 
 
-def test_token_resembles_flag():
+def test_is_potential_flag():
     parser = ACArgumentParser()
 
     # Not valid flags
-    assert not token_resembles_flag('', parser)
-    assert not token_resembles_flag('non-flag', parser)
-    assert not token_resembles_flag('-', parser)
-    assert not token_resembles_flag('--has space', parser)
-    assert not token_resembles_flag('-2', parser)
+    assert not is_potential_flag('', parser)
+    assert not is_potential_flag('non-flag', parser)
+    assert not is_potential_flag('-', parser)
+    assert not is_potential_flag('--has space', parser)
+    assert not is_potential_flag('-2', parser)
 
     # Valid flags
-    assert token_resembles_flag('-flag', parser)
-    assert token_resembles_flag('--flag', parser)
+    assert is_potential_flag('-flag', parser)
+    assert is_potential_flag('--flag', parser)
diff --git a/tests/test_autocompletion.py b/tests/test_autocompletion.py
index 3473ab38..7285af5c 100644
--- a/tests/test_autocompletion.py
+++ b/tests/test_autocompletion.py
@@ -279,3 +279,66 @@ def test_autcomp_custom_func_list_and_dict_arg(cmd2_app):
            cmd2_app.completion_matches == ['S01E02', 'S01E03', 'S02E01', 'S02E03']
 
 
+def test_argparse_remainder_flag_completion(cmd2_app):
+    import cmd2
+    import argparse
+
+    # Test flag completion as first arg of positional with nargs=argparse.REMAINDER
+    text = '--h'
+    line = 'help command {}'.format(text)
+    endidx = len(line)
+    begidx = endidx - len(text)
+
+    # --h should not complete into --help because we are in the argparse.REMAINDER section
+    assert complete_tester(text, line, begidx, endidx, cmd2_app) is None
+
+    # Test flag completion within an already started positional with nargs=argparse.REMAINDER
+    text = '--h'
+    line = 'help command subcommand {}'.format(text)
+    endidx = len(line)
+    begidx = endidx - len(text)
+
+    # --h should not complete into --help because we are in the argparse.REMAINDER section
+    assert complete_tester(text, line, begidx, endidx, cmd2_app) is None
+
+    # Test a flag with nargs=argparse.REMAINDER
+    parser = argparse.ArgumentParser()
+    parser.add_argument('-f', nargs=argparse.REMAINDER)
+
+    # Overwrite eof's parser for this test
+    cmd2.Cmd.do_eof.argparser = parser
+
+    text = '--h'
+    line = 'eof -f {}'.format(text)
+    endidx = len(line)
+    begidx = endidx - len(text)
+
+    # --h should not complete into --help because we are in the argparse.REMAINDER section
+    assert complete_tester(text, line, begidx, endidx, cmd2_app) is None
+
+
+def test_completion_after_double_dash(cmd2_app):
+    """
+    Test completion after --, which argparse says (all args after -- are non-options)
+    All of these tests occur outside of an argparse.REMAINDER section since those tests
+    are handled in test_argparse_remainder_flag_completion
+    """
+
+    # Test -- as the last token
+    text = '--'
+    line = 'help {}'.format(text)
+    endidx = len(line)
+    begidx = endidx - len(text)
+
+    # Since -- is the last token, then it should show flag choices
+    first_match = complete_tester(text, line, begidx, endidx, cmd2_app)
+    assert first_match is not None and '--help' in cmd2_app.completion_matches
+
+    # Test -- to end all flag completion
+    text = '--'
+    line = 'help -- {}'.format(text)
+    endidx = len(line)
+    begidx = endidx - len(text)
+
+    # Since -- appeared before the -- being completed, nothing should be completed
+    assert complete_tester(text, line, begidx, endidx, cmd2_app) is None
diff --git a/tests/test_completion.py b/tests/test_completion.py
index ed36eb01..0df06423 100644
--- a/tests/test_completion.py
+++ b/tests/test_completion.py
@@ -716,31 +716,6 @@ def test_add_opening_quote_delimited_space_in_prefix(cmd2_app):
            os.path.commonprefix(cmd2_app.completion_matches) == expected_common_prefix and \
            cmd2_app.display_matches == expected_display
 
-def test_argparse_remainder_completion(cmd2_app):
-    # First test a positional with nargs=argparse.REMAINDER
-    text = '--h'
-    line = 'help command subcommand {}'.format(text)
-    endidx = len(line)
-    begidx = endidx - len(text)
-
-    # --h should not complete into --help because we are in the argparse.REMAINDER sections
-    assert complete_tester(text, line, begidx, endidx, cmd2_app) is None
-
-    # Now test a flag with nargs=argparse.REMAINDER
-    parser = argparse.ArgumentParser()
-    parser.add_argument('-f', nargs=argparse.REMAINDER)
-
-    # Overwrite eof's parser for this test
-    cmd2.Cmd.do_eof.argparser = parser
-
-    text = '--h'
-    line = 'eof -f {}'.format(text)
-    endidx = len(line)
-    begidx = endidx - len(text)
-
-    # --h should not complete into --help because we are in the argparse.REMAINDER sections
-    assert complete_tester(text, line, begidx, endidx, cmd2_app) is None
-
 @pytest.fixture
 def sc_app():
     c = SubcommandsExample()
diff --git a/tests/test_pyscript.py b/tests/test_pyscript.py
index bcb72a3b..ce5e267d 100644
--- a/tests/test_pyscript.py
+++ b/tests/test_pyscript.py
@@ -238,9 +238,10 @@ def test_pyscript_custom_name(ps_echo, request):
 
 def test_pyscript_argparse_checks(ps_app, capsys):
     # Test command that has nargs.REMAINDER and make sure all tokens are accepted
-    run_cmd(ps_app, 'py app.alias.create("my_alias", "alias_command", "command_arg1", "command_arg2")')
+    # Include a flag in the REMAINDER section to show that they are processed as literals in that section
+    run_cmd(ps_app, 'py app.alias.create("my_alias", "alias_command", "command_arg1", "-h")')
     out = run_cmd(ps_app, 'alias list my_alias')
-    assert out == normalize('alias create my_alias alias_command command_arg1 command_arg2')
+    assert out == normalize('alias create my_alias alias_command command_arg1 -h')
 
     # Specify flag outside of keyword argument
     run_cmd(ps_app, 'py app.help("-h")')