Fixed unit test failures and addressed code review comments

Author: tleonhardt

CHANGELOG.md

@@ -10,7 +10,7 @@
     ArgparseFunctor should now be compliant with how argparse expects
     REMAINDER arguments to be ordered.
     * Fixed bugs with how AutoCompleter handles flag prefixes. It is no
-    longer hard-coded to use '-' and will check againstn the prefix_chars in
+    longer hard-coded to use '-' and will check against the prefix_chars in
     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.

cmd2/argparse_completer.py

@@ -335,7 +335,7 @@ def consume_flag_argument() -> None:
             """Consuming token as a flag argument"""
             # we're consuming flag arguments
             # if this is not empty and is not another potential flag, count towards flag arguments
-            # if the token is a single character length, it doesn't matter that it matches a flag prefix
+            # if the token is a single character, it doesn't matter whether it matches a flag prefix
             if token and (len(token) == 1 or token[0] not in self._parser.prefix_chars) and flag_action is not None:
                 flag_arg.count += 1
 
@@ -360,7 +360,7 @@ def consume_positional_argument() -> None:
                 consumed_arg_values[pos_action.dest].append(token)
 
         def process_action_nargs(action: argparse.Action, arg_state: AutoCompleter._ArgumentState) -> None:
-            """Process the current argparse Action and initialize the ArgumentState object used 
+            """Process the current argparse Action and initialize the ArgumentState object used
             to track what arguments we have processed for this action"""
             if isinstance(action, _RangeAction):
                 arg_state.min = action.nargs_min
@@ -389,16 +389,15 @@ def process_action_nargs(action: argparse.Action, arg_state: AutoCompleter._Argu
                     arg_state.min = action.nargs
                     arg_state.max = action.nargs
 
-
-        # This next block of processing tries to parse all parameters before the last parameter.  
+        # This next block of processing tries to parse all parameters before the last parameter.
         # We're trying to determine what specific argument the current cursor positition should be
         # matched with. When we finish parsing all of the arguments, we can determine whether the
         # last token is a positional or flag argument and which specific argument it is.
-        # 
-        # We're also trying to save every flag that has been used as well as every value that 
+        #
+        # We're also trying to save every flag that has been used as well as every value that
         # has been used for a positional or flag parameter.  By saving this information we can exclude
         # it from the completion results we generate for the last token. For example, single-use flag
-        # arguments will be hidden from the list of available flags. Also, arguments with a 
+        # arguments will be hidden from the list of available flags. Also, arguments with a
         # defined list of possible values will exclude values that have already been used.
 
         # notes when the last token has been reached
@@ -408,7 +407,7 @@ def process_action_nargs(action: argparse.Action, arg_state: AutoCompleter._Argu
             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 a remainder action is found, force all future tokens to go to that
                 if remainder['arg'] is not None:
                     if remainder['action'] == pos_action:
                         consume_positional_argument()
@@ -423,21 +422,21 @@ def process_action_nargs(action: argparse.Action, arg_state: AutoCompleter._Argu
                     #   - 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
+                    # 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 \
+                    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
 
-
                     # At this point we're no longer consuming flag arguments. Is the current argument a potential flag?
                     # If the argument is the start of a flag and this is the last token, we proceed forward to try
-                    # and match against our known flags. 
+                    # and match against our known flags.
                     # If this argument is not the last token and the argument is exactly a flag prefix, then this
                     # token should be consumed as an argument to a prior flag or positional argument.
-                    if len(token) > 0 and token[0] in self._parser.prefix_chars and not skip_flag and\
-                            (is_last_token or (not is_last_token and token not in self._parser.prefix_chars)):
+                    if len(token) > 0 and token[0] in self._parser.prefix_chars and not skip_flag and \
+                            (is_last_token or token not in self._parser.prefix_chars):
                         # reset some tracking values
                         flag_arg.reset()
                         # don't reset positional tracking because flags can be interspersed anywhere between positionals
@@ -512,7 +511,8 @@ def process_action_nargs(action: argparse.Action, arg_state: AutoCompleter._Argu
         # 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:
+        if not flag_arg.needed and len(tokens[-1]) > 0 and tokens[-1][0] in self._parser.prefix_chars and \
+                remainder['arg'] is None:
             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

cmd2/pyscript_bridge.py

@@ -232,8 +232,8 @@ def process_flag(action, value):
                 # If this is a flag parameter that can accept a variable number of arguments and we have not
                 # reached the max number, add a list completion suffix to tell argparse to move to the next
                 # parameter
-                if action.option_strings and isinstance(action, _RangeAction) \
-                        and action.nargs_max > len(value):
+                if action.option_strings and isinstance(action, _RangeAction) and action.nargs_max is not None and \
+                        action.nargs_max > len(value):
                     cmd_str[0] += '{0}{0} '.format(self._parser.prefix_chars[0])
 
             else:
@@ -245,8 +245,8 @@ def process_flag(action, value):
                 # If this is a flag parameter that can accept a variable number of arguments and we have not
                 # reached the max number, add a list completion suffix to tell argparse to move to the next
                 # parameter
-                if action.option_strings and isinstance(action, _RangeAction) \
-                        and action.nargs_max > 1:
+                if action.option_strings and isinstance(action, _RangeAction) and action.nargs_max is not None and \
+                        action.nargs_max > 1:
                     cmd_str[0] += '{0}{0} '.format(self._parser.prefix_chars[0])
 
         def process_action(action):
@@ -269,7 +269,8 @@ def traverse_parser(parser):
                     process_action(action)
             # next process positional arguments
             for action in parser._actions:
-                if action.dest in self._args and action.dest not in self._flag_args and action.dest != self._remainder_arg:
+                if action.dest in self._args and action.dest not in self._flag_args and \
+                        action.dest != self._remainder_arg:
                     process_action(action)
             # Keep remainder argument last
             for action in parser._actions:

tests/test_autocompletion.py

@@ -36,7 +36,7 @@ def cmd2_app():
 
 MEDIA_MOVIES_ADD_HELP = '''Usage: media movies add -d DIRECTOR{1..2}
                         [-h]
-                        title {G, PG, PG-13, R, NC-17} [actor [...]]
+                        title {G, PG, PG-13, R, NC-17} ...
 
 positional arguments:
   title                 Movie Title