From 0ad057143b326f78398fbc9a3840d9d6b378e0be Mon Sep 17 00:00:00 2001 From: Conor Sheehan Date: Fri, 9 Dec 2022 20:31:12 +0000 Subject: [PATCH] Fix #317 Help text short args (#318) * Add short args to help text for all types of kwargs. * Updates tests to reflect new help text. --- fire/helptext.py | 58 +++++++++++++++++++++++++++++++++++------ fire/helptext_test.py | 36 ++++++++++++++++++------- fire/test_components.py | 10 +++++++ 3 files changed, 87 insertions(+), 17 deletions(-) diff --git a/fire/helptext.py b/fire/helptext.py index 331b6649..6e7fbb07 100644 --- a/fire/helptext.py +++ b/fire/helptext.py @@ -33,6 +33,7 @@ from __future__ import division from __future__ import print_function +import collections import itertools import sys @@ -172,9 +173,25 @@ def _DescriptionSection(component, info): return None -def _CreateKeywordOnlyFlagItem(flag, docstring_info, spec): +def _CreateKeywordOnlyFlagItem(flag, docstring_info, spec, short_arg): return _CreateFlagItem( - flag, docstring_info, spec, required=flag not in spec.kwonlydefaults) + flag, docstring_info, spec, required=flag not in spec.kwonlydefaults, + short_arg=short_arg) + + +def _GetShortFlags(flags): + """Gets a list of single-character flags that uniquely identify a flag. + + 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 = collections.Counter(short_flags) + return [v for v in short_flags if short_flag_counts[v] == 1] def _ArgsAndFlagsSections(info, spec, metadata): @@ -209,25 +226,47 @@ def _ArgsAndFlagsSections(info, spec, metadata): ('NOTES', 'You can also use flags syntax for POSITIONAL ARGUMENTS') ) + unique_short_args = _GetShortFlags(args_with_defaults) positional_flag_items = [ - _CreateFlagItem(flag, docstring_info, spec, required=False) + _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) + _CreateKeywordOnlyFlagItem( + flag, docstring_info, spec, + short_arg=flag[0] in unique_short_kwonly_flags + ) 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}' documented_kwargs = [] - for flag in docstring_info.args or []: + flag_string = '--{name}' + short_flag_string = '-{short_name}, --{name}' + + # add short flags if possible + flags = docstring_info.args or [] + 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_flags: + 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 @@ -422,7 +461,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: @@ -434,6 +473,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. """ @@ -455,6 +495,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_flag}, '.format(short_flag=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 14e0874a..404d9812 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 --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 +95,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 +123,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 +142,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 +159,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 +287,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.') @@ -294,7 +296,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) @testutils.skipIf( six.PY2, @@ -374,7 +376,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(), @@ -389,7 +391,8 @@ 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): @@ -435,6 +438,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, --first', + help_screen) + self.assertIn('\n --last', help_screen) + self.assertIn('\n --late', help_screen) + + class UsageTest(testutils.BaseTestCase): diff --git a/fire/test_components.py b/fire/test_components.py index 027a6b19..5fcb056e 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(first='first', last='last', late='late'): + """Function with kwarg and defaults. + + :key first: Description of first. + :key last: Description of last. + :key late: Description of late. + """ + del last, late + return first