From 6c2c1fcb792f401832c5a1bd4bc8268de748c225 Mon Sep 17 00:00:00 2001 From: greenmoon55 Date: Thu, 12 Oct 2017 21:59:38 -0400 Subject: [PATCH 1/3] Do not treat arguments that start with '--' as string. --- fire/core.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/fire/core.py b/fire/core.py index a7e3adaa..652a1761 100644 --- a/fire/core.py +++ b/fire/core.py @@ -565,7 +565,7 @@ def _MakeParseFn(fn): def _ParseFn(args): """Parses the list of `args` into (varargs, kwargs), remaining_args.""" - kwargs, remaining_args = _ParseKeywordArgs(args, all_args, fn_spec.varkw) + kwargs, unconsumed_named_args, remaining_args = _ParseKeywordArgs(args, all_args, fn_spec.varkw) # Note: _ParseArgs modifies kwargs. parsed_args, kwargs, remaining_args, capacity = _ParseArgs( @@ -594,6 +594,7 @@ def _ParseFn(args): varargs[index] = _ParseValue(value, None, None, metadata) varargs = parsed_args + varargs + remaining_args += unconsumed_named_args consumed_args = args[:len(args) - len(remaining_args)] return (varargs, kwargs), consumed_args, remaining_args, capacity @@ -684,6 +685,7 @@ def _ParseKeywordArgs(args, fn_args, fn_keywords): remaining_args: A list of the unused arguments from the original args. """ kwargs = {} + unconsumed_named_args = [] if args: remaining_args = [] skip_argument = False @@ -730,7 +732,9 @@ def _ParseKeywordArgs(args, fn_args, fn_keywords): if got_argument and (keyword in fn_args or fn_keywords): kwargs[keyword] = value skip_argument = not contains_equals and not is_bool_syntax - arg_consumed = True + else: + unconsumed_named_args.append(argument) + arg_consumed = True if not arg_consumed: # The argument was not consumed, so it is still a remaining argument. @@ -738,7 +742,7 @@ def _ParseKeywordArgs(args, fn_args, fn_keywords): else: remaining_args = args - return kwargs, remaining_args + return kwargs, unconsumed_named_args, remaining_args def _ParseValue(value, index, arg, metadata): From 5c5d238dd5bd540b3b5a0b1c1123d7e7fee47764 Mon Sep 17 00:00:00 2001 From: greenmoon55 Date: Sun, 3 Dec 2017 13:02:25 -0500 Subject: [PATCH 2/3] Fixes kwarg value bug and modify test case for arg starting with '--' --- fire/core.py | 22 +++++++++++++--------- fire/fire_test.py | 7 +++++-- 2 files changed, 18 insertions(+), 11 deletions(-) diff --git a/fire/core.py b/fire/core.py index 652a1761..09268c84 100644 --- a/fire/core.py +++ b/fire/core.py @@ -565,7 +565,7 @@ def _MakeParseFn(fn): def _ParseFn(args): """Parses the list of `args` into (varargs, kwargs), remaining_args.""" - kwargs, unconsumed_named_args, remaining_args = _ParseKeywordArgs(args, all_args, fn_spec.varkw) + kwargs, remaining_kwargs, remaining_args = _ParseKeywordArgs(args, all_args, fn_spec.varkw) # Note: _ParseArgs modifies kwargs. parsed_args, kwargs, remaining_args, capacity = _ParseArgs( @@ -594,7 +594,7 @@ def _ParseFn(args): varargs[index] = _ParseValue(value, None, None, metadata) varargs = parsed_args + varargs - remaining_args += unconsumed_named_args + remaining_args += remaining_kwargs consumed_args = args[:len(args) - len(remaining_args)] return (varargs, kwargs), consumed_args, remaining_args, capacity @@ -682,10 +682,11 @@ def _ParseKeywordArgs(args, fn_args, fn_keywords): fn_keywords: The argument name for **kwargs, or None if **kwargs not used Returns: kwargs: A dictionary mapping keywords to values. + remaining_kwargs: A list of the unused kwargs from the original args. remaining_args: A list of the unused arguments from the original args. """ kwargs = {} - unconsumed_named_args = [] + remaining_kwargs = [] if args: remaining_args = [] skip_argument = False @@ -729,12 +730,15 @@ def _ParseKeywordArgs(args, fn_args, fn_keywords): # In order for us to consume the argument as a keyword arg, we either: # Need to be explicitly expecting the keyword, or we need to be # accepting **kwargs. - if got_argument and (keyword in fn_args or fn_keywords): - kwargs[keyword] = value + if got_argument: skip_argument = not contains_equals and not is_bool_syntax - else: - unconsumed_named_args.append(argument) - arg_consumed = True + arg_consumed = True + if keyword in fn_args or fn_keywords: + kwargs[keyword] = value + else: + remaining_kwargs.append(argument) + if skip_argument: + remaining_kwargs.append(args[index + 1]) if not arg_consumed: # The argument was not consumed, so it is still a remaining argument. @@ -742,7 +746,7 @@ def _ParseKeywordArgs(args, fn_args, fn_keywords): else: remaining_args = args - return kwargs, unconsumed_named_args, remaining_args + return kwargs, remaining_kwargs, remaining_args def _ParseValue(value, index, arg, metadata): diff --git a/fire/fire_test.py b/fire/fire_test.py index 89ccb38a..88235608 100644 --- a/fire/fire_test.py +++ b/fire/fire_test.py @@ -368,9 +368,12 @@ def testBoolParsingLessExpectedCases(self): fire.Fire(tc.MixedDefaults, command=['identity', 'True', '10']), (True, 10)) - # Note: Does not return ('--test', '0'). + # Note: Does not return (True, '--test') or ('--test', 0). + with self.assertRaisesFireExit(2): + fire.Fire(tc.MixedDefaults, command=['identity', '--alpha', '--test']) + self.assertEqual(fire.Fire(tc.MixedDefaults, - command=['identity', '--alpha', '--test']), + command=['identity', '--alpha', 'True', '"--test"']), (True, '--test')) # To get ('--test', '0'), use one of the following: self.assertEqual(fire.Fire(tc.MixedDefaults, From a572ad7f677c4c1fb06f6edf34c6e709420dfc67 Mon Sep 17 00:00:00 2001 From: greenmoon55 Date: Sun, 3 Dec 2017 13:18:16 -0500 Subject: [PATCH 3/3] Fix pylint errors --- fire/core.py | 116 +++++++++++++++++++++++----------------------- fire/fire_test.py | 10 ++-- 2 files changed, 65 insertions(+), 61 deletions(-) diff --git a/fire/core.py b/fire/core.py index 09268c84..5dc40d0a 100644 --- a/fire/core.py +++ b/fire/core.py @@ -565,7 +565,8 @@ def _MakeParseFn(fn): def _ParseFn(args): """Parses the list of `args` into (varargs, kwargs), remaining_args.""" - kwargs, remaining_kwargs, remaining_args = _ParseKeywordArgs(args, all_args, fn_spec.varkw) + kwargs, remaining_kwargs, remaining_args = \ + _ParseKeywordArgs(args, all_args, fn_spec.varkw) # Note: _ParseArgs modifies kwargs. parsed_args, kwargs, remaining_args, capacity = _ParseArgs( @@ -687,64 +688,65 @@ def _ParseKeywordArgs(args, fn_args, fn_keywords): """ kwargs = {} remaining_kwargs = [] - if args: - remaining_args = [] - skip_argument = False - - for index, argument in enumerate(args): - if skip_argument: - skip_argument = False - continue - - arg_consumed = False - if argument.startswith('--'): - # This is a named argument; get its value from this arg or the next. - got_argument = False - - keyword = argument[2:] - contains_equals = '=' in keyword - is_bool_syntax = ( - not contains_equals and - (index + 1 == len(args) or args[index + 1].startswith('--'))) - if contains_equals: - keyword, value = keyword.split('=', 1) - got_argument = True - elif is_bool_syntax: - # Since there's no next arg or the next arg is a Flag, we consider - # this flag to be a boolean. + remaining_args = [] + + if not args: + return kwargs, remaining_kwargs, remaining_args + + skip_argument = False + + for index, argument in enumerate(args): + if skip_argument: + skip_argument = False + continue + + arg_consumed = False + if argument.startswith('--'): + # This is a named argument; get its value from this arg or the next. + got_argument = False + + keyword = argument[2:] + contains_equals = '=' in keyword + is_bool_syntax = ( + not contains_equals and + (index + 1 == len(args) or args[index + 1].startswith('--'))) + if contains_equals: + keyword, value = keyword.split('=', 1) + got_argument = True + elif is_bool_syntax: + # Since there's no next arg or the next arg is a Flag, we consider + # this flag to be a boolean. + got_argument = True + if keyword in fn_args: + value = 'True' + elif keyword.startswith('no'): + keyword = keyword[2:] + value = 'False' + else: + value = 'True' + else: + if index + 1 < len(args): + value = args[index + 1] got_argument = True - if keyword in fn_args: - value = 'True' - elif keyword.startswith('no'): - keyword = keyword[2:] - value = 'False' - else: - value = 'True' + + keyword = keyword.replace('-', '_') + + # In order for us to consume the argument as a keyword arg, we either: + # Need to be explicitly expecting the keyword, or we need to be + # accepting **kwargs. + if got_argument: + skip_argument = not contains_equals and not is_bool_syntax + arg_consumed = True + if keyword in fn_args or fn_keywords: + kwargs[keyword] = value else: - if index + 1 < len(args): - value = args[index + 1] - got_argument = True - - keyword = keyword.replace('-', '_') - - # In order for us to consume the argument as a keyword arg, we either: - # Need to be explicitly expecting the keyword, or we need to be - # accepting **kwargs. - if got_argument: - skip_argument = not contains_equals and not is_bool_syntax - arg_consumed = True - if keyword in fn_args or fn_keywords: - kwargs[keyword] = value - else: - remaining_kwargs.append(argument) - if skip_argument: - remaining_kwargs.append(args[index + 1]) - - if not arg_consumed: - # The argument was not consumed, so it is still a remaining argument. - remaining_args.append(argument) - else: - remaining_args = args + remaining_kwargs.append(argument) + if skip_argument: + remaining_kwargs.append(args[index + 1]) + + if not arg_consumed: + # The argument was not consumed, so it is still a remaining argument. + remaining_args.append(argument) return kwargs, remaining_kwargs, remaining_args diff --git a/fire/fire_test.py b/fire/fire_test.py index 88235608..8f1f53ec 100644 --- a/fire/fire_test.py +++ b/fire/fire_test.py @@ -370,11 +370,13 @@ def testBoolParsingLessExpectedCases(self): # Note: Does not return (True, '--test') or ('--test', 0). with self.assertRaisesFireExit(2): - fire.Fire(tc.MixedDefaults, command=['identity', '--alpha', '--test']) + fire.Fire(tc.MixedDefaults, command=['identity', '--alpha', '--test']) - self.assertEqual(fire.Fire(tc.MixedDefaults, - command=['identity', '--alpha', 'True', '"--test"']), - (True, '--test')) + self.assertEqual( + fire.Fire( + tc.MixedDefaults, + command=['identity', '--alpha', 'True', '"--test"']), + (True, '--test')) # To get ('--test', '0'), use one of the following: self.assertEqual(fire.Fire(tc.MixedDefaults, command=['identity', '--alpha=--test']),