From edc3ae8c9428d03a32806fb430561ae97a97ffda Mon Sep 17 00:00:00 2001 From: Conor Sheehan Date: Fri, 5 Feb 2021 21:39:45 +0000 Subject: [PATCH 1/8] add short args to help text --- fire/helptext.py | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/fire/helptext.py b/fire/helptext.py index b1d10b44..72835973 100644 --- a/fire/helptext.py +++ b/fire/helptext.py @@ -35,6 +35,7 @@ import itertools import sys +from collections import Counter from fire import completion from fire import custom_descriptions @@ -222,12 +223,24 @@ def _ArgsAndFlagsSections(info, spec, metadata): if spec.varkw: # Include kwargs documented via :key param: flag_string = '--{name}' + short_flag_string = '-{short_name}, --{name}' documented_kwargs = [] - for flag in docstring_info.args or []: + + # add short flags if possible + flags = docstring_info.args or [] + short_flags = list(map(lambda f: f.name[0], flags)) + short_flag_counts = Counter(short_flags) + unique_short_args = [v for v in short_flags if short_flag_counts[v] == 1] + for flag in flags: if isinstance(flag, docstrings.KwargInfo): + if flag.name[0] in unique_short_args: + flag_string = short_flag_string.format(name=flag.name, short_name=flag.name[0]) + else: + flag_string = flag_string.format(name=flag.name) + flag_item = _CreateFlagItem( flag.name, docstring_info, spec, - flag_string=flag_string.format(name=flag.name)) + flag_string=flag_string) documented_kwargs.append(flag_item) if documented_kwargs: # Separate documented kwargs from other flags using a message From 9aef7f5a6e3e0a0ffc9eaf268cc4ba282e79a535 Mon Sep 17 00:00:00 2001 From: Conor Sheehan Date: Fri, 5 Feb 2021 22:08:02 +0000 Subject: [PATCH 2/8] short args for all types of kwargs, start updating tests. --- fire/helptext.py | 58 +++++++++++++++++++++++++++++++++---------- fire/helptext_test.py | 6 ++--- 2 files changed, 48 insertions(+), 16 deletions(-) diff --git a/fire/helptext.py b/fire/helptext.py index 72835973..f5136701 100644 --- a/fire/helptext.py +++ b/fire/helptext.py @@ -173,9 +173,25 @@ def _DescriptionSection(component, info): return None -def _CreateKeywordOnlyFlagItem(flag, docstring_info, spec): +def _CreateKeywordOnlyFlagItem(flag, docstring_info, spec, flag_string=None): return _CreateFlagItem( - flag, docstring_info, spec, required=flag not in spec.kwonlydefaults) + flag, docstring_info, spec, required=flag not in spec.kwonlydefaults, + flag_string=flag_string + ) + + +def _GetShortFlags(flags): + """ + Args: + flags: list of strings representing flags + + Returns: + List of single character short flags, + where the character occurred at the start of a flag once. + """ + short_flags = [f[0] for f in flags] + short_flag_counts = Counter(short_flags) + return [v for v in short_flags if short_flag_counts[v] == 1] def _ArgsAndFlagsSections(info, spec, metadata): @@ -210,30 +226,46 @@ def _ArgsAndFlagsSections(info, spec, metadata): ('NOTES', 'You can also use flags syntax for POSITIONAL ARGUMENTS') ) - positional_flag_items = [ - _CreateFlagItem(flag, docstring_info, spec, required=False) - for flag in args_with_defaults - ] + flag_string = '--{name}' + short_flag_string = '-{short_name}, --{name}' + positional_flag_items = [] + unique_short_args = _GetShortFlags(args_with_defaults) + for flag in args_with_defaults: + if flag[0] in unique_short_args: + positional_flag_items.append( + _CreateFlagItem( + flag, docstring_info, spec, required=False, + flag_string=short_flag_string.format(name=flag, short_name=flag[0]) + ) + ) + else: + positional_flag_items.append( + _CreateFlagItem(flag, docstring_info, spec, required=False) + ) + + unique_short_kwonly_flags = _GetShortFlags(spec.kwonlyargs) kwonly_flag_items = [ - _CreateKeywordOnlyFlagItem(flag, docstring_info, spec) + _CreateKeywordOnlyFlagItem( + flag, docstring_info, spec, + flag_string=short_flag_string.format(name=flag, short_name=flag[0]) + ) + if flag[0] in unique_short_kwonly_flags + else CreateKeywordOnlyFlagItem(flag, docstring_info, spec) for flag in spec.kwonlyargs ] flag_items = positional_flag_items + kwonly_flag_items if spec.varkw: # Include kwargs documented via :key param: - flag_string = '--{name}' - short_flag_string = '-{short_name}, --{name}' documented_kwargs = [] # add short flags if possible flags = docstring_info.args or [] - short_flags = list(map(lambda f: f.name[0], flags)) - short_flag_counts = Counter(short_flags) - unique_short_args = [v for v in short_flags if short_flag_counts[v] == 1] + flag_names = [f.name for f in flags] + unique_short_flags = _GetShortFlags(flag_names) for flag in flags: if isinstance(flag, docstrings.KwargInfo): - if flag.name[0] in unique_short_args: + if flag.name[0] in unique_short_flags: flag_string = short_flag_string.format(name=flag.name, short_name=flag.name[0]) else: flag_string = flag_string.format(name=flag.name) diff --git a/fire/helptext_test.py b/fire/helptext_test.py index 29250cec..3b81a93f 100644 --- a/fire/helptext_test.py +++ b/fire/helptext_test.py @@ -294,7 +294,7 @@ def testHelpTextKeywordOnlyArgumentsWithoutDefault(self): output = helptext.HelpText( component=component, trace=trace.FireTrace(component, 'double')) self.assertIn('NAME\n double', output) - self.assertIn('FLAGS\n --count=COUNT (required)', output) + self.assertIn('FLAGS\n -c, --count=COUNT (required)', output) def testHelpScreen(self): component = tc.ClassWithDocstring() @@ -362,7 +362,7 @@ def testHelpScreenForFunctionFunctionWithDefaultArgs(self): Returns the input multiplied by 2. FLAGS - --count=COUNT + -c, --count=COUNT Default: 0 Input number that you want to double.""" self.assertEqual(textwrap.dedent(expected_output).strip(), @@ -377,7 +377,7 @@ def testHelpTextUnderlineFlag(self): formatting.Bold('SYNOPSIS') + '\n triple ', help_screen) self.assertIn( - formatting.Bold('FLAGS') + '\n --' + formatting.Underline('count'), + formatting.Bold('FLAGS') + '\n -c, --' + formatting.Underline('count'), help_screen) def testHelpTextBoldCommandName(self): From 48cf12836b7c9fc0f487cf79400e4e8b5a597c27 Mon Sep 17 00:00:00 2001 From: Conor Sheehan Date: Fri, 5 Feb 2021 22:30:38 +0000 Subject: [PATCH 3/8] refactor _CreateFlagItem, update tests --- fire/helptext.py | 37 ++++++++++++++++--------------------- fire/helptext_test.py | 12 ++++++------ 2 files changed, 22 insertions(+), 27 deletions(-) diff --git a/fire/helptext.py b/fire/helptext.py index f5136701..6933015e 100644 --- a/fire/helptext.py +++ b/fire/helptext.py @@ -173,10 +173,10 @@ def _DescriptionSection(component, info): return None -def _CreateKeywordOnlyFlagItem(flag, docstring_info, spec, flag_string=None): +def _CreateKeywordOnlyFlagItem(flag, docstring_info, spec, short_arg): return _CreateFlagItem( flag, docstring_info, spec, required=flag not in spec.kwonlydefaults, - flag_string=flag_string + short_arg=short_arg ) @@ -226,31 +226,21 @@ def _ArgsAndFlagsSections(info, spec, metadata): ('NOTES', 'You can also use flags syntax for POSITIONAL ARGUMENTS') ) - flag_string = '--{name}' - short_flag_string = '-{short_name}, --{name}' - positional_flag_items = [] unique_short_args = _GetShortFlags(args_with_defaults) - for flag in args_with_defaults: - if flag[0] in unique_short_args: - positional_flag_items.append( - _CreateFlagItem( - flag, docstring_info, spec, required=False, - flag_string=short_flag_string.format(name=flag, short_name=flag[0]) - ) - ) - else: - positional_flag_items.append( - _CreateFlagItem(flag, docstring_info, spec, required=False) - ) + positional_flag_items = [ + _CreateFlagItem( + flag, docstring_info, spec, required=False, + short_arg=flag[0] in unique_short_args + ) + for flag in args_with_defaults + ] unique_short_kwonly_flags = _GetShortFlags(spec.kwonlyargs) kwonly_flag_items = [ _CreateKeywordOnlyFlagItem( flag, docstring_info, spec, - flag_string=short_flag_string.format(name=flag, short_name=flag[0]) + short_arg=flag[0] in unique_short_kwonly_flags ) - if flag[0] in unique_short_kwonly_flags - else CreateKeywordOnlyFlagItem(flag, docstring_info, spec) for flag in spec.kwonlyargs ] flag_items = positional_flag_items + kwonly_flag_items @@ -258,6 +248,8 @@ def _ArgsAndFlagsSections(info, spec, metadata): if spec.varkw: # Include kwargs documented via :key param: documented_kwargs = [] + flag_string = '--{name}' + short_flag_string = '-{short_name}, --{name}' # add short flags if possible flags = docstring_info.args or [] @@ -467,7 +459,7 @@ def _CreateArgItem(arg, docstring_info, spec): def _CreateFlagItem(flag, docstring_info, spec, required=False, - flag_string=None): + flag_string=None, short_arg=False): """Returns a string describing a flag using docstring and FullArgSpec info. Args: @@ -479,6 +471,7 @@ def _CreateFlagItem(flag, docstring_info, spec, required=False, required: Whether the flag is required. flag_string: If provided, use this string for the flag, rather than constructing one from the flag name. + short_arg: Whether the flag has a short variation or not. Returns: A string to be used in constructing the help screen for the function. """ @@ -500,6 +493,8 @@ def _CreateFlagItem(flag, docstring_info, spec, required=False, flag_name_upper=formatting.Underline(flag.upper())) if required: flag_string += ' (required)' + if short_arg: + flag_string = '-{short_arg}, '.format(short_arg=flag[0]) + flag_string arg_type = _GetArgType(flag, spec) arg_default = _GetArgDefault(flag, spec) diff --git a/fire/helptext_test.py b/fire/helptext_test.py index 3b81a93f..9f4b37f7 100644 --- a/fire/helptext_test.py +++ b/fire/helptext_test.py @@ -81,7 +81,7 @@ def testHelpTextFunctionWithDefaults(self): self.assertIn('NAME\n triple', help_screen) self.assertIn('SYNOPSIS\n triple ', help_screen) self.assertNotIn('DESCRIPTION', help_screen) - self.assertIn('FLAGS\n --count=COUNT\n Default: 0', help_screen) + self.assertIn('FLAGS\n -c, --count=COUNT\n Default: 0', help_screen) self.assertNotIn('NOTES', help_screen) def testHelpTextFunctionWithLongDefaults(self): @@ -93,7 +93,7 @@ def testHelpTextFunctionWithLongDefaults(self): self.assertIn('SYNOPSIS\n text ', help_screen) self.assertNotIn('DESCRIPTION', help_screen) self.assertIn( - 'FLAGS\n --string=STRING\n' + 'FLAGS\n -s, --string=STRING\n' ' Default: \'0001020304050607080910' '1112131415161718192021222324252627282...', help_screen) @@ -121,7 +121,7 @@ def testHelpTextFunctionWithKwargsAndDefaults(self): self.assertIn('SYNOPSIS\n text ARG1 ARG2 ', help_screen) self.assertIn('DESCRIPTION\n Function with kwarg', help_screen) self.assertIn( - 'FLAGS\n --opt=OPT\n Default: True\n' + 'FLAGS\n -o, --opt=OPT\n Default: True\n' ' The following flags are also accepted.' '\n --arg3\n Description of arg3.\n ' 'Additional undocumented flags may also be accepted.', @@ -140,7 +140,7 @@ def testHelpTextFunctionWithDefaultsAndTypes(self): self.assertIn('SYNOPSIS\n double ', help_screen) self.assertIn('DESCRIPTION', help_screen) self.assertIn( - 'FLAGS\n --count=COUNT\n Type: float\n Default: 0', + 'FLAGS\n -c, --count=COUNT\n Type: float\n Default: 0', help_screen) self.assertNotIn('NOTES', help_screen) @@ -157,7 +157,7 @@ def testHelpTextFunctionWithTypesAndDefaultNone(self): self.assertIn('SYNOPSIS\n get_int ', help_screen) self.assertNotIn('DESCRIPTION', help_screen) self.assertIn( - 'FLAGS\n --value=VALUE\n' + 'FLAGS\n -v, --value=VALUE\n' ' Type: Optional[int]\n Default: None', help_screen) self.assertNotIn('NOTES', help_screen) @@ -285,7 +285,7 @@ def testHelpTextKeywordOnlyArgumentsWithDefault(self): output = helptext.HelpText( component=component, trace=trace.FireTrace(component, 'with_default')) self.assertIn('NAME\n with_default', output) - self.assertIn('FLAGS\n --x=X', output) + self.assertIn('FLAGS\n -x, --x=X', output) @testutils.skipIf( six.PY2, 'Python 2 does not support keyword-only arguments.') From 93a4e0cac8ba30dffa841a2acd2b0466ddf2458f Mon Sep 17 00:00:00 2001 From: Conor Sheehan Date: Fri, 5 Feb 2021 22:56:40 +0000 Subject: [PATCH 4/8] fix pylint offences --- fire/helptext.py | 4 +++- fire/helptext_test.py | 7 +++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/fire/helptext.py b/fire/helptext.py index 6933015e..726c22db 100644 --- a/fire/helptext.py +++ b/fire/helptext.py @@ -258,7 +258,9 @@ def _ArgsAndFlagsSections(info, spec, metadata): for flag in flags: if isinstance(flag, docstrings.KwargInfo): if flag.name[0] in unique_short_flags: - flag_string = short_flag_string.format(name=flag.name, short_name=flag.name[0]) + flag_string = short_flag_string.format( + name=flag.name, short_name=flag.name[0] + ) else: flag_string = flag_string.format(name=flag.name) diff --git a/fire/helptext_test.py b/fire/helptext_test.py index 9f4b37f7..84e08cee 100644 --- a/fire/helptext_test.py +++ b/fire/helptext_test.py @@ -81,7 +81,9 @@ def testHelpTextFunctionWithDefaults(self): self.assertIn('NAME\n triple', help_screen) self.assertIn('SYNOPSIS\n triple ', help_screen) self.assertNotIn('DESCRIPTION', help_screen) - self.assertIn('FLAGS\n -c, --count=COUNT\n Default: 0', help_screen) + self.assertIn( + 'FLAGS\n -c, --count=COUNT\n Default: 0', + help_screen) self.assertNotIn('NOTES', help_screen) def testHelpTextFunctionWithLongDefaults(self): @@ -377,7 +379,8 @@ def testHelpTextUnderlineFlag(self): formatting.Bold('SYNOPSIS') + '\n triple ', help_screen) self.assertIn( - formatting.Bold('FLAGS') + '\n -c, --' + formatting.Underline('count'), + formatting.Bold('FLAGS') + '\n -c, --' + + formatting.Underline('count'), help_screen) def testHelpTextBoldCommandName(self): From 1a921891f802ea04c5f19c767573f6d049d4dc7f Mon Sep 17 00:00:00 2001 From: Conor Sheehan Date: Fri, 5 Feb 2021 23:14:06 +0000 Subject: [PATCH 5/8] add test case for example in initial issue --- fire/helptext_test.py | 15 +++++++++++++++ fire/test_components.py | 10 ++++++++++ 2 files changed, 25 insertions(+) diff --git a/fire/helptext_test.py b/fire/helptext_test.py index 84e08cee..25e0436a 100644 --- a/fire/helptext_test.py +++ b/fire/helptext_test.py @@ -426,6 +426,21 @@ def testHelpTextNameSectionCommandWithSeparatorVerbose(self): self.assertIn('double -', help_screen) self.assertIn('double - -', help_screen) + def testHelpTextMultipleKeywoardArgumentsWithShortArgs(self): + component = tc.fn_with_multiple_defaults + t = trace.FireTrace(component, name='shortargs') + help_screen = helptext.HelpText(component, t) + self.assertIn(formatting.Bold('NAME') + '\n shortargs', help_screen) + self.assertIn( + formatting.Bold('SYNOPSIS') + '\n shortargs ', + help_screen) + self.assertIn( + formatting.Bold('FLAGS') + '\n -f, --foo', + help_screen) + self.assertIn('\n --bar', help_screen) + self.assertIn('\n --baz', help_screen) + + class UsageTest(testutils.BaseTestCase): diff --git a/fire/test_components.py b/fire/test_components.py index eee9a07c..821495f8 100644 --- a/fire/test_components.py +++ b/fire/test_components.py @@ -563,3 +563,13 @@ def fn_with_kwarg_and_defaults(arg1, arg2, opt=True, **kwargs): del arg1, arg2, opt return kwargs.get('arg3') # pylint: enable=g-doc-args,g-doc-return-or-yield + +def fn_with_multiple_defaults(foo='foo', bar='bar', baz='baz'): + """Function with kwarg and defaults. + + :key foo: Description of foo. + :key bar: Description of bar. + :key baz: Description of baz. + """ + del bar, baz + return foo From 4d7162b6eb8efa2d84bae41f14935fd014cb8567 Mon Sep 17 00:00:00 2001 From: Conor Sheehan Date: Fri, 5 Feb 2021 23:37:50 +0000 Subject: [PATCH 6/8] fix pylint blacklisted names --- fire/helptext_test.py | 6 +++--- fire/test_components.py | 12 ++++++------ 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/fire/helptext_test.py b/fire/helptext_test.py index 25e0436a..53329c4c 100644 --- a/fire/helptext_test.py +++ b/fire/helptext_test.py @@ -435,10 +435,10 @@ def testHelpTextMultipleKeywoardArgumentsWithShortArgs(self): formatting.Bold('SYNOPSIS') + '\n shortargs ', help_screen) self.assertIn( - formatting.Bold('FLAGS') + '\n -f, --foo', + formatting.Bold('FLAGS') + '\n -f, --first', help_screen) - self.assertIn('\n --bar', help_screen) - self.assertIn('\n --baz', help_screen) + self.assertIn('\n --last', help_screen) + self.assertIn('\n --late', help_screen) diff --git a/fire/test_components.py b/fire/test_components.py index 821495f8..a02ab1b2 100644 --- a/fire/test_components.py +++ b/fire/test_components.py @@ -564,12 +564,12 @@ def fn_with_kwarg_and_defaults(arg1, arg2, opt=True, **kwargs): return kwargs.get('arg3') # pylint: enable=g-doc-args,g-doc-return-or-yield -def fn_with_multiple_defaults(foo='foo', bar='bar', baz='baz'): +def fn_with_multiple_defaults(first='first', last='last', late='late'): """Function with kwarg and defaults. - :key foo: Description of foo. - :key bar: Description of bar. - :key baz: Description of baz. + :key first: Description of first. + :key last: Description of last. + :key late: Description of late. """ - del bar, baz - return foo + del last, late + return first From d95c76e8e739d2361ab31a2f6cc073735ea6039f Mon Sep 17 00:00:00 2001 From: Conor Sheehan Date: Sat, 6 Feb 2021 00:00:39 +0000 Subject: [PATCH 7/8] fix pylint line continutation for py2.7 --- fire/helptext.py | 19 +++++++++---------- fire/helptext_test.py | 4 ++-- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/fire/helptext.py b/fire/helptext.py index 726c22db..bc9d6c82 100644 --- a/fire/helptext.py +++ b/fire/helptext.py @@ -176,8 +176,7 @@ def _DescriptionSection(component, info): def _CreateKeywordOnlyFlagItem(flag, docstring_info, spec, short_arg): return _CreateFlagItem( flag, docstring_info, spec, required=flag not in spec.kwonlydefaults, - short_arg=short_arg - ) + short_arg=short_arg) def _GetShortFlags(flags): @@ -228,18 +227,18 @@ def _ArgsAndFlagsSections(info, spec, metadata): unique_short_args = _GetShortFlags(args_with_defaults) positional_flag_items = [ - _CreateFlagItem( - flag, docstring_info, spec, required=False, - short_arg=flag[0] in unique_short_args - ) - for flag in args_with_defaults + _CreateFlagItem( + flag, docstring_info, spec, required=False, + short_arg=flag[0] in unique_short_args + ) + for flag in args_with_defaults ] unique_short_kwonly_flags = _GetShortFlags(spec.kwonlyargs) kwonly_flag_items = [ _CreateKeywordOnlyFlagItem( - flag, docstring_info, spec, - short_arg=flag[0] in unique_short_kwonly_flags + flag, docstring_info, spec, + short_arg=flag[0] in unique_short_kwonly_flags ) for flag in spec.kwonlyargs ] @@ -259,7 +258,7 @@ def _ArgsAndFlagsSections(info, spec, metadata): if isinstance(flag, docstrings.KwargInfo): if flag.name[0] in unique_short_flags: flag_string = short_flag_string.format( - name=flag.name, short_name=flag.name[0] + name=flag.name, short_name=flag.name[0] ) else: flag_string = flag_string.format(name=flag.name) diff --git a/fire/helptext_test.py b/fire/helptext_test.py index 53329c4c..7c210bee 100644 --- a/fire/helptext_test.py +++ b/fire/helptext_test.py @@ -82,8 +82,8 @@ def testHelpTextFunctionWithDefaults(self): self.assertIn('SYNOPSIS\n triple ', help_screen) self.assertNotIn('DESCRIPTION', help_screen) self.assertIn( - 'FLAGS\n -c, --count=COUNT\n Default: 0', - help_screen) + 'FLAGS\n -c, --count=COUNT\n Default: 0', + help_screen) self.assertNotIn('NOTES', help_screen) def testHelpTextFunctionWithLongDefaults(self): From 6988eb8df1e1f8c3a7e423d93082595a28c4238b Mon Sep 17 00:00:00 2001 From: David Bieber Date: Fri, 9 Dec 2022 15:28:36 -0500 Subject: [PATCH 8/8] Clean-up of short flags logic in preparation for merge --- fire/helptext.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/fire/helptext.py b/fire/helptext.py index bc9d6c82..f61e4649 100644 --- a/fire/helptext.py +++ b/fire/helptext.py @@ -33,9 +33,9 @@ from __future__ import division from __future__ import print_function +import collections import itertools import sys -from collections import Counter from fire import completion from fire import custom_descriptions @@ -180,7 +180,8 @@ def _CreateKeywordOnlyFlagItem(flag, docstring_info, spec, short_arg): def _GetShortFlags(flags): - """ + """Gets a list of single-character flags that uniquely identify a flag. + Args: flags: list of strings representing flags @@ -189,7 +190,7 @@ def _GetShortFlags(flags): where the character occurred at the start of a flag once. """ short_flags = [f[0] for f in flags] - short_flag_counts = Counter(short_flags) + short_flag_counts = collections.Counter(short_flags) return [v for v in short_flags if short_flag_counts[v] == 1] @@ -495,7 +496,7 @@ def _CreateFlagItem(flag, docstring_info, spec, required=False, if required: flag_string += ' (required)' if short_arg: - flag_string = '-{short_arg}, '.format(short_arg=flag[0]) + flag_string + flag_string = '-{short_flag}, '.format(short_flag=flag[0]) + flag_string arg_type = _GetArgType(flag, spec) arg_default = _GetArgDefault(flag, spec)