From 96bcfe012b3b1659dbd0244ac6c4a43dfb5328d6 Mon Sep 17 00:00:00 2001 From: Eric Lin Date: Sat, 6 Oct 2018 17:17:17 +0000 Subject: Added handling of nargs=argparse.REMAINDER in both AutoCompleter and ArgparseFunctor Should correctly force all subsequent arguments to go to the REMAINDER argument once it is detected. Re-arranged the command generation in ArgparseFunctor to print flag arguments before positionals Also forces the remainder arguments to always be last. --- cmd2/pyscript_bridge.py | 65 +++++++++++++++++++++++++++++++++++++------------ 1 file changed, 49 insertions(+), 16 deletions(-) (limited to 'cmd2/pyscript_bridge.py') diff --git a/cmd2/pyscript_bridge.py b/cmd2/pyscript_bridge.py index a70a7ae6..7920e1be 100644 --- a/cmd2/pyscript_bridge.py +++ b/cmd2/pyscript_bridge.py @@ -75,6 +75,10 @@ class ArgparseFunctor: # Dictionary mapping command argument name to value self._args = {} + # tag the argument that's a remainder type + self._remainder_arg = None + # separately track flag arguments so they will be printed before positionals + self._flag_args = [] # argparse object for the current command layer self.__current_subcommand_parser = parser @@ -109,7 +113,6 @@ class ArgparseFunctor: next_pos_index = 0 has_subcommand = False - consumed_kw = [] # Iterate through the current sub-command's arguments in order for action in self.__current_subcommand_parser._actions: @@ -118,7 +121,7 @@ class ArgparseFunctor: # this is a flag argument, search for the argument by name in the parameters if action.dest in kwargs: self._args[action.dest] = kwargs[action.dest] - consumed_kw.append(action.dest) + self._flag_args.append(action.dest) else: # This is a positional argument, search the positional arguments passed in. if not isinstance(action, argparse._SubParsersAction): @@ -157,6 +160,10 @@ class ArgparseFunctor: elif action.nargs == '*': self._args[action.dest] = args[next_pos_index:next_pos_index + pos_remain] next_pos_index += pos_remain + elif action.nargs == argparse.REMAINDER: + self._args[action.dest] = args[next_pos_index:next_pos_index + pos_remain] + next_pos_index += pos_remain + self._remainder_arg = action.dest elif action.nargs == '?': self._args[action.dest] = args[next_pos_index] next_pos_index += 1 @@ -168,7 +175,7 @@ class ArgparseFunctor: # Check if there are any extra arguments we don't know how to handle for kw in kwargs: - if kw not in self._args: # consumed_kw: + if kw not in self._args: raise TypeError("{}() got an unexpected keyword argument '{}'".format( self.__current_subcommand_parser.prog, kw)) @@ -214,27 +221,53 @@ class ArgparseFunctor: if ' ' in item: item = '"{}"'.format(item) cmd_str[0] += '{} '.format(item) + + # 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): + cmd_str[0] += '{0}{0} '.format(self._parser.prefix_chars[0]) + else: value = str(value).strip() if ' ' in value: value = '"{}"'.format(value) cmd_str[0] += '{} '.format(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: + cmd_str[0] += '{0}{0} '.format(self._parser.prefix_chars[0]) + + def process_action(action): + if isinstance(action, argparse._SubParsersAction): + cmd_str[0] += '{} '.format(self._args[action.dest]) + traverse_parser(action.choices[self._args[action.dest]]) + elif isinstance(action, argparse._AppendAction): + if isinstance(self._args[action.dest], list) or isinstance(self._args[action.dest], tuple): + for values in self._args[action.dest]: + process_flag(action, values) + else: + process_flag(action, self._args[action.dest]) + else: + process_flag(action, self._args[action.dest]) + def traverse_parser(parser): + # first process optional flag arguments for action in parser._actions: - # was something provided for the argument - if action.dest in self._args: - if isinstance(action, argparse._SubParsersAction): - cmd_str[0] += '{} '.format(self._args[action.dest]) - traverse_parser(action.choices[self._args[action.dest]]) - elif isinstance(action, argparse._AppendAction): - if isinstance(self._args[action.dest], list) or isinstance(self._args[action.dest], tuple): - for values in self._args[action.dest]: - process_flag(action, values) - else: - process_flag(action, self._args[action.dest]) - else: - process_flag(action, self._args[action.dest]) + if action.dest in self._args and action.dest in self._flag_args and action.dest != self._remainder_arg: + 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: + process_action(action) + # Keep remainder argument last + for action in parser._actions: + if action.dest in self._args and action.dest == self._remainder_arg: + process_action(action) traverse_parser(self._parser) -- cgit v1.2.1 From 5fbbc53939b5cf48894e6853852211ec036c7e02 Mon Sep 17 00:00:00 2001 From: Todd Leonhardt Date: Sat, 6 Oct 2018 18:19:48 -0400 Subject: Fixed unit test failures and addressed code review comments --- cmd2/pyscript_bridge.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) (limited to 'cmd2/pyscript_bridge.py') diff --git a/cmd2/pyscript_bridge.py b/cmd2/pyscript_bridge.py index 8187d7fd..5f5c3542 100644 --- a/cmd2/pyscript_bridge.py +++ b/cmd2/pyscript_bridge.py @@ -232,8 +232,8 @@ class ArgparseFunctor: # 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 @@ class ArgparseFunctor: # 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 @@ class ArgparseFunctor: 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: -- cgit v1.2.1 From 5e6cf76bd85de2ca63e2c78d5c4a865dc69d63c2 Mon Sep 17 00:00:00 2001 From: Kevin Van Brunt Date: Sat, 6 Oct 2018 22:00:48 -0400 Subject: Quoting strings with utility function --- cmd2/pyscript_bridge.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) (limited to 'cmd2/pyscript_bridge.py') diff --git a/cmd2/pyscript_bridge.py b/cmd2/pyscript_bridge.py index 5f5c3542..9a1d0fec 100644 --- a/cmd2/pyscript_bridge.py +++ b/cmd2/pyscript_bridge.py @@ -13,7 +13,7 @@ import sys from typing import List, Callable, Optional from .argparse_completer import _RangeAction -from .utils import namedtuple_with_defaults, StdSim +from .utils import namedtuple_with_defaults, StdSim, quote_string_if_needed # Python 3.4 require contextlib2 for temporarily redirecting stderr and stdout if sys.version_info < (3, 5): @@ -225,8 +225,7 @@ class ArgparseFunctor: if isinstance(value, List) or isinstance(value, tuple): for item in value: item = str(item).strip() - if ' ' in item: - item = '"{}"'.format(item) + item = quote_string_if_needed(item) cmd_str[0] += '{} '.format(item) # If this is a flag parameter that can accept a variable number of arguments and we have not @@ -238,8 +237,7 @@ class ArgparseFunctor: else: value = str(value).strip() - if ' ' in value: - value = '"{}"'.format(value) + value = quote_string_if_needed(value) cmd_str[0] += '{} '.format(value) # If this is a flag parameter that can accept a variable number of arguments and we have not -- cgit v1.2.1 From 8be2db480a7445fe18af7aba4fbd78922c747de3 Mon Sep 17 00:00:00 2001 From: Kevin Van Brunt Date: Sat, 6 Oct 2018 22:30:16 -0400 Subject: Added check to prevent optional value strings from being accepted as positionals --- cmd2/pyscript_bridge.py | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) (limited to 'cmd2/pyscript_bridge.py') diff --git a/cmd2/pyscript_bridge.py b/cmd2/pyscript_bridge.py index 9a1d0fec..30fdb170 100644 --- a/cmd2/pyscript_bridge.py +++ b/cmd2/pyscript_bridge.py @@ -201,7 +201,18 @@ class ArgparseFunctor: # reconstruct the cmd2 command from the python call cmd_str = [''] - def process_flag(action, value): + def has_optional_prefix(arg: str) -> bool: + """ + Checks if an argument value begins with prefix characters intended for argparse optional values + :param arg: argument value being checked + :return: True if arg begins with one of the prefix characters + """ + for char in self._parser.prefix_chars: + if arg.startswith(char): + return True + return False + + def process_argument(action, value): if isinstance(action, argparse._CountAction): if isinstance(value, int): for _ in range(value): @@ -221,10 +232,16 @@ class ArgparseFunctor: # was the argument a flag? if action.option_strings: cmd_str[0] += '{} '.format(action.option_strings[0]) + is_flag = True + else: + is_flag = False if isinstance(value, List) or isinstance(value, tuple): for item in value: item = str(item).strip() + if not is_flag and has_optional_prefix(item): + raise ValueError('Value provided for {} ({}) appears to be an optional'. + format(action.dest, item)) item = quote_string_if_needed(item) cmd_str[0] += '{} '.format(item) @@ -237,6 +254,8 @@ class ArgparseFunctor: else: value = str(value).strip() + if not is_flag and has_optional_prefix(value): + raise ValueError('Value provided for {} ({}) appears to be an optional'.format(action.dest, value)) value = quote_string_if_needed(value) cmd_str[0] += '{} '.format(value) @@ -254,11 +273,11 @@ class ArgparseFunctor: elif isinstance(action, argparse._AppendAction): if isinstance(self._args[action.dest], list) or isinstance(self._args[action.dest], tuple): for values in self._args[action.dest]: - process_flag(action, values) + process_argument(action, values) else: - process_flag(action, self._args[action.dest]) + process_argument(action, self._args[action.dest]) else: - process_flag(action, self._args[action.dest]) + process_argument(action, self._args[action.dest]) def traverse_parser(parser): # first process optional flag arguments -- cgit v1.2.1 From 4a136b3cfb3d7177303cb6f67edccb7b72fa91b1 Mon Sep 17 00:00:00 2001 From: Kevin Van Brunt Date: Sun, 7 Oct 2018 14:19:27 -0400 Subject: Allowing negative number values when checking for optional prefix characters --- cmd2/pyscript_bridge.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) (limited to 'cmd2/pyscript_bridge.py') diff --git a/cmd2/pyscript_bridge.py b/cmd2/pyscript_bridge.py index 30fdb170..5b2d3e46 100644 --- a/cmd2/pyscript_bridge.py +++ b/cmd2/pyscript_bridge.py @@ -204,9 +204,13 @@ class ArgparseFunctor: def has_optional_prefix(arg: str) -> bool: """ Checks if an argument value begins with prefix characters intended for argparse optional values + Allows anything that appears to be a negative number :param arg: argument value being checked :return: True if arg begins with one of the prefix characters """ + if self._parser._negative_number_matcher.match(arg): + return False + for char in self._parser.prefix_chars: if arg.startswith(char): return True @@ -232,14 +236,11 @@ class ArgparseFunctor: # was the argument a flag? if action.option_strings: cmd_str[0] += '{} '.format(action.option_strings[0]) - is_flag = True - else: - is_flag = False if isinstance(value, List) or isinstance(value, tuple): for item in value: item = str(item).strip() - if not is_flag and has_optional_prefix(item): + if has_optional_prefix(item): raise ValueError('Value provided for {} ({}) appears to be an optional'. format(action.dest, item)) item = quote_string_if_needed(item) @@ -254,7 +255,7 @@ class ArgparseFunctor: else: value = str(value).strip() - if not is_flag and has_optional_prefix(value): + if has_optional_prefix(value): raise ValueError('Value provided for {} ({}) appears to be an optional'.format(action.dest, value)) value = quote_string_if_needed(value) cmd_str[0] += '{} '.format(value) -- cgit v1.2.1 From ad3ca3263e2a3552b010d17458a51697f405720f Mon Sep 17 00:00:00 2001 From: Kevin Van Brunt Date: Sun, 7 Oct 2018 17:33:05 -0400 Subject: Use argparser to determine if a token looks like an optional --- cmd2/pyscript_bridge.py | 19 ++----------------- 1 file changed, 2 insertions(+), 17 deletions(-) (limited to 'cmd2/pyscript_bridge.py') diff --git a/cmd2/pyscript_bridge.py b/cmd2/pyscript_bridge.py index 5b2d3e46..037c7a77 100644 --- a/cmd2/pyscript_bridge.py +++ b/cmd2/pyscript_bridge.py @@ -201,21 +201,6 @@ class ArgparseFunctor: # reconstruct the cmd2 command from the python call cmd_str = [''] - def has_optional_prefix(arg: str) -> bool: - """ - Checks if an argument value begins with prefix characters intended for argparse optional values - Allows anything that appears to be a negative number - :param arg: argument value being checked - :return: True if arg begins with one of the prefix characters - """ - if self._parser._negative_number_matcher.match(arg): - return False - - for char in self._parser.prefix_chars: - if arg.startswith(char): - return True - return False - def process_argument(action, value): if isinstance(action, argparse._CountAction): if isinstance(value, int): @@ -240,7 +225,7 @@ class ArgparseFunctor: if isinstance(value, List) or isinstance(value, tuple): for item in value: item = str(item).strip() - if has_optional_prefix(item): + if self._parser._parse_optional(item) is not None: raise ValueError('Value provided for {} ({}) appears to be an optional'. format(action.dest, item)) item = quote_string_if_needed(item) @@ -255,7 +240,7 @@ class ArgparseFunctor: else: value = str(value).strip() - if has_optional_prefix(value): + if self._parser._parse_optional(value) is not None: raise ValueError('Value provided for {} ({}) appears to be an optional'.format(action.dest, value)) value = quote_string_if_needed(value) cmd_str[0] += '{} '.format(value) -- cgit v1.2.1 From 9b1bfab6e2a908d119e46bcc70e98d94739ab4a4 Mon Sep 17 00:00:00 2001 From: Kevin Van Brunt Date: Tue, 9 Oct 2018 18:10:55 -0400 Subject: Added ability for argcompleter to determine difference between flag and negative number --- cmd2/pyscript_bridge.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) (limited to 'cmd2/pyscript_bridge.py') diff --git a/cmd2/pyscript_bridge.py b/cmd2/pyscript_bridge.py index 037c7a77..11a2cbb3 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 +from .argparse_completer import _RangeAction, token_resembles_flag from .utils import namedtuple_with_defaults, StdSim, quote_string_if_needed # Python 3.4 require contextlib2 for temporarily redirecting stderr and stdout @@ -225,9 +225,9 @@ class ArgparseFunctor: if isinstance(value, List) or isinstance(value, tuple): for item in value: item = str(item).strip() - if self._parser._parse_optional(item) is not None: - raise ValueError('Value provided for {} ({}) appears to be an optional'. - format(action.dest, item)) + if token_resembles_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) cmd_str[0] += '{} '.format(item) @@ -240,8 +240,9 @@ class ArgparseFunctor: else: value = str(value).strip() - if self._parser._parse_optional(value) is not None: - raise ValueError('Value provided for {} ({}) appears to be an optional'.format(action.dest, value)) + if token_resembles_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) cmd_str[0] += '{} '.format(value) -- cgit v1.2.1