From 74561e0bba6d6aa5762e20c4ae3e99754a84bb34 Mon Sep 17 00:00:00 2001 From: Burt Bielicki Date: Fri, 29 Apr 2016 10:56:10 -0700 Subject: [PATCH 01/14] show global params at the bottom of the help output --- src/azure/cli/_help.py | 12 +++++++++--- src/azure/cli/application.py | 12 ++++++------ src/azure/cli/extensions/query.py | 6 +++--- 3 files changed, 18 insertions(+), 12 deletions(-) diff --git a/src/azure/cli/_help.py b/src/azure/cli/_help.py index 2cb0f423833..c1ca6b3e21d 100644 --- a/src/azure/cli/_help.py +++ b/src/azure/cli/_help.py @@ -75,9 +75,13 @@ def print_arguments(help_file): required_tag = L(' [Required]') max_name_length = max(len(p.name) + (len(required_tag) if p.required else 0) for p in help_file.parameters) - for p in sorted(help_file.parameters, key=lambda p: str(not p.required) + p.name): + added_global_space = False + for p in sorted(help_file.parameters, key=lambda p: str(not p.required) + str(p.global_param) + p.name): indent = 1 required_text = required_tag if p.required else '' + if p.global_param and not added_global_space: + print('') + added_global_space = True _print_indent('{0}{1}{2}{3}'.format(p.name, _get_column_indent(p.name + required_text, max_name_length), @@ -204,7 +208,8 @@ def __init__(self, delimiters, parser): for action in [a for a in parser._actions if a.help != argparse.SUPPRESS]: # pylint: disable=protected-access self.parameters.append(HelpParameter(' '.join(sorted(action.option_strings)), action.help, - required=action.required)) + required=action.required, + global_param='global' in action.dest or action.dest == 'help')) def _load_from_data(self, data): super(CommandHelpFile, self)._load_from_data(data) @@ -229,13 +234,14 @@ def _load_from_data(self, data): class HelpParameter(object): #pylint: disable=too-few-public-methods - def __init__(self, param_name, description, required): + def __init__(self, param_name, description, required, global_param=False): self.name = param_name self.required = required self.type = 'string' self.short_summary = description self.long_summary = '' self.value_sources = [] + self.global_param = global_param def update_from_data(self, data): if self.name != data.get('name'): diff --git a/src/azure/cli/application.py b/src/azure/cli/application.py index d2b710984d2..5134a4ae165 100644 --- a/src/azure/cli/application.py +++ b/src/azure/cli/application.py @@ -135,16 +135,16 @@ def _enable_autocomplete(parser): @staticmethod def _register_builtin_arguments(parser): - parser.add_argument('--subscription', dest='_subscription_id', help=argparse.SUPPRESS) - parser.add_argument('--output', '-o', dest='_output_format', + parser.add_argument('--subscription', dest='_subscription_id_global', help=argparse.SUPPRESS) + parser.add_argument('--output', '-o', dest='_output_format_global', choices=['list', 'json', 'tsv'], help='Output format of type "list", "json" or "tsv"') # The arguments for verbosity don't get parsed by argparse but we add it here for help. - parser.add_argument('--verbose', dest='_log_verbosity_verbose', + parser.add_argument('--verbose', dest='_log_verbosity_verbose_global', help='Increase logging verbosity. Use --debug for full debug logs.') - parser.add_argument('--debug', dest='_log_verbosity_debug', + parser.add_argument('--debug', dest='_log_verbosity_debug_global', help='Increase logging verbosity to show all debug logs.') def _handle_builtin_arguments(self, args): - self.configuration.output_format = args._output_format #pylint: disable=protected-access - del args._output_format + self.configuration.output_format = args._output_format_global #pylint: disable=protected-access + del args._output_format_global diff --git a/src/azure/cli/extensions/query.py b/src/azure/cli/extensions/query.py index 46aa8302301..57fc5cded48 100644 --- a/src/azure/cli/extensions/query.py +++ b/src/azure/cli/extensions/query.py @@ -2,14 +2,14 @@ def _register_global_parameter(parser): # Let the program know that we are adding a parameter --query - parser.add_argument('--query', dest='_jmespath_query', metavar='JMESPATH', + parser.add_argument('--query', dest='_jmespath_query_global', metavar='JMESPATH', help='JMESPath query string. See http://jmespath.org/ for more information and examples.') # pylint: disable=line-too-long def register(application): def handle_query_parameter(args): try: - query_value = args._jmespath_query # pylint: disable=protected-access - del args._jmespath_query + query_value = args._jmespath_query_global # pylint: disable=protected-access + del args._jmespath_query_global if query_value: def filter_output(event_data): From 58901d83b85708634731f46bdaaa259a2711f884 Mon Sep 17 00:00:00 2001 From: Burt Bielicki Date: Fri, 29 Apr 2016 11:06:01 -0700 Subject: [PATCH 02/14] add label for globals --- src/azure/cli/_help.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/azure/cli/_help.py b/src/azure/cli/_help.py index c1ca6b3e21d..5428fc9fe3d 100644 --- a/src/azure/cli/_help.py +++ b/src/azure/cli/_help.py @@ -80,7 +80,7 @@ def print_arguments(help_file): indent = 1 required_text = required_tag if p.required else '' if p.global_param and not added_global_space: - print('') + print('\nGlobal Arguments') added_global_space = True _print_indent('{0}{1}{2}{3}'.format(p.name, _get_column_indent(p.name + required_text, From 84f1e4a8a7330454e4c7b5126670216b32e401eb Mon Sep 17 00:00:00 2001 From: Burt Bielicki Date: Fri, 29 Apr 2016 12:50:22 -0700 Subject: [PATCH 03/14] sort globals based on argument group --- src/azure/cli/_help.py | 2 +- src/azure/cli/application.py | 10 ++++++---- src/azure/cli/extensions/query.py | 3 ++- src/azure/cli/parser.py | 5 +++++ 4 files changed, 14 insertions(+), 6 deletions(-) diff --git a/src/azure/cli/_help.py b/src/azure/cli/_help.py index 5428fc9fe3d..b9f9e6cd1fd 100644 --- a/src/azure/cli/_help.py +++ b/src/azure/cli/_help.py @@ -209,7 +209,7 @@ def __init__(self, delimiters, parser): self.parameters.append(HelpParameter(' '.join(sorted(action.option_strings)), action.help, required=action.required, - global_param='global' in action.dest or action.dest == 'help')) + global_param=action.container.description == parser.get_global_group().description or action.dest == 'help')) def _load_from_data(self, data): super(CommandHelpFile, self)._load_from_data(data) diff --git a/src/azure/cli/application.py b/src/azure/cli/application.py index 5134a4ae165..7da51513959 100644 --- a/src/azure/cli/application.py +++ b/src/azure/cli/application.py @@ -135,14 +135,16 @@ def _enable_autocomplete(parser): @staticmethod def _register_builtin_arguments(parser): - parser.add_argument('--subscription', dest='_subscription_id_global', help=argparse.SUPPRESS) - parser.add_argument('--output', '-o', dest='_output_format_global', + global_group = parser.get_global_group() + + global_group.add_argument('--subscription', dest='_subscription_id', help=argparse.SUPPRESS) + global_group.add_argument('--output', '-o', dest='_output_format', choices=['list', 'json', 'tsv'], help='Output format of type "list", "json" or "tsv"') # The arguments for verbosity don't get parsed by argparse but we add it here for help. - parser.add_argument('--verbose', dest='_log_verbosity_verbose_global', + global_group.add_argument('--verbose', dest='_log_verbosity_verbose', help='Increase logging verbosity. Use --debug for full debug logs.') - parser.add_argument('--debug', dest='_log_verbosity_debug_global', + global_group.add_argument('--debug', dest='_log_verbosity_debug', help='Increase logging verbosity to show all debug logs.') def _handle_builtin_arguments(self, args): diff --git a/src/azure/cli/extensions/query.py b/src/azure/cli/extensions/query.py index 57fc5cded48..2d763b16e9f 100644 --- a/src/azure/cli/extensions/query.py +++ b/src/azure/cli/extensions/query.py @@ -2,7 +2,8 @@ def _register_global_parameter(parser): # Let the program know that we are adding a parameter --query - parser.add_argument('--query', dest='_jmespath_query_global', metavar='JMESPATH', + global_group = parser.get_global_group() + global_group.add_argument('--query', dest='_jmespath_query', metavar='JMESPATH', help='JMESPath query string. See http://jmespath.org/ for more information and examples.') # pylint: disable=line-too-long def register(application): diff --git a/src/azure/cli/parser.py b/src/azure/cli/parser.py index 095107e2efd..b92b4664d57 100644 --- a/src/azure/cli/parser.py +++ b/src/azure/cli/parser.py @@ -17,6 +17,11 @@ def __init__(self, **kwargs): self.help_file = kwargs.pop('help_file', None) super(AzCliCommandParser, self).__init__(**kwargs) + def get_global_group(self): + if len(self._action_groups) < 3: + self.add_argument_group('global', 'global arguments') + return self._action_groups[2] + def load_command_table(self, command_table): """Load a command table into our parser. """ From e3d83fbc34720d1c91090ef24a97e79a41ef9930 Mon Sep 17 00:00:00 2001 From: Burt Bielicki Date: Fri, 29 Apr 2016 13:43:13 -0700 Subject: [PATCH 04/14] add 'az' to the beginning of commands and groups --- src/azure/cli/_help.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/azure/cli/_help.py b/src/azure/cli/_help.py index b9f9e6cd1fd..e95827c04dc 100644 --- a/src/azure/cli/_help.py +++ b/src/azure/cli/_help.py @@ -105,7 +105,7 @@ def _print_header(help_file): _print_indent(L('Command') if help_file.type == 'command' else L('Group'), indent) indent += 1 - _print_indent('{0}{1}'.format(help_file.command, + _print_indent('{0}{1}'.format('az ' + help_file.command, ': ' + help_file.short_summary if help_file.short_summary else ''), From 86a4ba4e54463565657c60197d95f3a79097ef82 Mon Sep 17 00:00:00 2001 From: Burt Bielicki Date: Fri, 29 Apr 2016 14:04:39 -0700 Subject: [PATCH 05/14] update tests, remove lint, minor help output change --- src/azure/cli/_help.py | 9 ++++++--- src/azure/cli/application.py | 13 +++++++------ src/azure/cli/extensions/query.py | 6 +++--- src/azure/cli/tests/test_help.py | 22 +++++++++++++--------- 4 files changed, 29 insertions(+), 21 deletions(-) diff --git a/src/azure/cli/_help.py b/src/azure/cli/_help.py index e95827c04dc..c45e9d2b25e 100644 --- a/src/azure/cli/_help.py +++ b/src/azure/cli/_help.py @@ -76,7 +76,8 @@ def print_arguments(help_file): max_name_length = max(len(p.name) + (len(required_tag) if p.required else 0) for p in help_file.parameters) added_global_space = False - for p in sorted(help_file.parameters, key=lambda p: str(not p.required) + str(p.global_param) + p.name): + for p in sorted(help_file.parameters, + key=lambda p: str(not p.required) + str(p.global_param) + p.name): indent = 1 required_text = required_tag if p.required else '' if p.global_param and not added_global_space: @@ -95,7 +96,6 @@ def print_arguments(help_file): _print_indent('{0}'.format(p.long_summary.rstrip()), indent) if p.value_sources: - _print_indent('') _print_indent(L("Values from: {0}").format(', '.join(p.value_sources)), indent) return indent @@ -130,6 +130,7 @@ def _print_groups(help_file): def _print_examples(help_file): indent = 0 + print('') _print_indent(L('Examples'), indent) for e in help_file.examples: @@ -209,7 +210,9 @@ def __init__(self, delimiters, parser): self.parameters.append(HelpParameter(' '.join(sorted(action.option_strings)), action.help, required=action.required, - global_param=action.container.description == parser.get_global_group().description or action.dest == 'help')) + global_param=action.container.description + == parser.get_global_group().description + or action.dest == 'help')) def _load_from_data(self, data): super(CommandHelpFile, self)._load_from_data(data) diff --git a/src/azure/cli/application.py b/src/azure/cli/application.py index 7da51513959..f50d8f50c21 100644 --- a/src/azure/cli/application.py +++ b/src/azure/cli/application.py @@ -139,14 +139,15 @@ def _register_builtin_arguments(parser): global_group.add_argument('--subscription', dest='_subscription_id', help=argparse.SUPPRESS) global_group.add_argument('--output', '-o', dest='_output_format', - choices=['list', 'json', 'tsv'], - help='Output format of type "list", "json" or "tsv"') + choices=['list', 'json', 'tsv'], + help='Output format of type "list", "json" or "tsv"') # The arguments for verbosity don't get parsed by argparse but we add it here for help. global_group.add_argument('--verbose', dest='_log_verbosity_verbose', - help='Increase logging verbosity. Use --debug for full debug logs.') + help='Increase logging verbosity.' + ' Use --debug for full debug logs.') global_group.add_argument('--debug', dest='_log_verbosity_debug', - help='Increase logging verbosity to show all debug logs.') + help='Increase logging verbosity to show all debug logs.') def _handle_builtin_arguments(self, args): - self.configuration.output_format = args._output_format_global #pylint: disable=protected-access - del args._output_format_global + self.configuration.output_format = args._output_format #pylint: disable=protected-access + del args._output_format diff --git a/src/azure/cli/extensions/query.py b/src/azure/cli/extensions/query.py index 2d763b16e9f..b38ecba26aa 100644 --- a/src/azure/cli/extensions/query.py +++ b/src/azure/cli/extensions/query.py @@ -4,13 +4,13 @@ def _register_global_parameter(parser): # Let the program know that we are adding a parameter --query global_group = parser.get_global_group() global_group.add_argument('--query', dest='_jmespath_query', metavar='JMESPATH', - help='JMESPath query string. See http://jmespath.org/ for more information and examples.') # pylint: disable=line-too-long + help='JMESPath query string. See http://jmespath.org/ for more information and examples.') # pylint: disable=line-too-long def register(application): def handle_query_parameter(args): try: - query_value = args._jmespath_query_global # pylint: disable=protected-access - del args._jmespath_query_global + query_value = args._jmespath_query # pylint: disable=protected-access + del args._jmespath_query if query_value: def filter_output(event_data): diff --git a/src/azure/cli/tests/test_help.py b/src/azure/cli/tests/test_help.py index 149cde05f92..08ad429b742 100644 --- a/src/azure/cli/tests/test_help.py +++ b/src/azure/cli/tests/test_help.py @@ -106,7 +106,7 @@ def test_handler(args): with self.assertRaises(SystemExit): app.execute('n1 -h'.split()) - self.assertEqual(True, io.getvalue().startswith('\nCommand\n n1\n long description')) + self.assertEqual(True, io.getvalue().startswith('\nCommand\n az n1\n long description')) @redirect_io def test_help_long_description_and_short_description(self): @@ -131,7 +131,7 @@ def test_handler(args): with self.assertRaises(SystemExit): app.execute('n1 -h'.split()) - self.assertEqual(True, io.getvalue().startswith('\nCommand\n n1: short description\n long description')) + self.assertEqual(True, io.getvalue().startswith('\nCommand\n az n1: short description\n long description')) @redirect_io def test_help_docstring_description_overrides_short_description(self): @@ -185,7 +185,7 @@ def test_handler(args): with self.assertRaises(SystemExit): app.execute('n1 -h'.split()) - self.assertEqual(True, io.getvalue().startswith('\nCommand\n n1\n line1\n line2')) + self.assertEqual(True, io.getvalue().startswith('\nCommand\n az n1\n line1\n line2')) @redirect_io @mock.patch('azure.cli.application.Application.register', return_value=None) @@ -228,16 +228,17 @@ def test_handler(args): app.execute('n1 -h'.split()) s = ''' Command - n1 + az n1 Arguments --foobar2 -fb2 [Required]: one line partial sentence paragraph(s) --foobar -fb : one line partial sentence text, markdown, etc. - Values from: az vm list, default --foobar3 -fb3 : the foobar3 + +Global Arguments --help -h : show this help message and exit ''' self.assertEqual(s, io.getvalue()) @@ -289,7 +290,7 @@ def test_handler(args): app.execute('n1 -h'.split()) s = ''' Command - n1: this module does xyz one-line or so + az n1: this module does xyz one-line or so this module.... kjsdflkj... klsfkj paragraph1 this module.... kjsdflkj... klsfkj paragraph2 @@ -298,9 +299,11 @@ def test_handler(args): paragraph(s) --foobar -fb : one line partial sentence text, markdown, etc. - Values from: az vm list, default + +Global Arguments --help -h : show this help message and exit + Examples foo example example details @@ -439,7 +442,7 @@ def test_handler2(args): with self.assertRaises(SystemExit): app.execute('group1 -h'.split()) - s = '\nGroup\n group1\n\nSub-Commands\n group2\n group3\n\n' + s = '\nGroup\n az group1\n\nSub-Commands\n group2\n group3\n\n' self.assertEqual(s, io.getvalue()) # Will uncomment when all errors are shown at once (help behaviors implementation) task #115631559 @@ -516,13 +519,14 @@ def test_handler(args): app.execute('test_group1 test_group2 --help'.split()) s = ''' Group - test_group1 test_group2: this module does xyz one-line or so + az test_group1 test_group2: this module does xyz one-line or so this module.... kjsdflkj... klsfkj paragraph1 this module.... kjsdflkj... klsfkj paragraph2 Sub-Commands n1: this module does xyz one-line or so + Examples foo example example details From aba66c0fb1f8f841377d2ea1454d102da8ac548e Mon Sep 17 00:00:00 2001 From: Burt Bielicki Date: Mon, 2 May 2016 09:05:08 -0700 Subject: [PATCH 06/14] pass in global group instead of global parser to global param handlers --- src/azure/cli/_help.py | 7 ++++--- src/azure/cli/application.py | 7 +++---- src/azure/cli/extensions/query.py | 3 +-- src/azure/cli/parser.py | 5 ----- 4 files changed, 8 insertions(+), 14 deletions(-) diff --git a/src/azure/cli/_help.py b/src/azure/cli/_help.py index c45e9d2b25e..3f6978ecba2 100644 --- a/src/azure/cli/_help.py +++ b/src/azure/cli/_help.py @@ -210,9 +210,7 @@ def __init__(self, delimiters, parser): self.parameters.append(HelpParameter(' '.join(sorted(action.option_strings)), action.help, required=action.required, - global_param=action.container.description - == parser.get_global_group().description - or action.dest == 'help')) + global_param=self.is_global(action))) def _load_from_data(self, data): super(CommandHelpFile, self)._load_from_data(data) @@ -235,6 +233,9 @@ def _load_from_data(self, data): raise HelpAuthoringException('Extra help param {0}'.format(extra_param['name'])) self.parameters = loaded_params + def is_global(self, action): + return action.container.description == 'global arguments' \ + or action.dest == 'help' class HelpParameter(object): #pylint: disable=too-few-public-methods def __init__(self, param_name, description, required, global_param=False): diff --git a/src/azure/cli/application.py b/src/azure/cli/application.py index f50d8f50c21..f410c91f413 100644 --- a/src/azure/cli/application.py +++ b/src/azure/cli/application.py @@ -50,7 +50,8 @@ def __init__(self, configuration): azure.cli.extensions.register_extensions(self) self.global_parser = AzCliCommandParser(prog='az', add_help=False) - self.raise_event(self.GLOBAL_PARSER_CREATED, self.global_parser) + global_group = self.global_parser.add_argument_group('global', 'global arguments') + self.raise_event(self.GLOBAL_PARSER_CREATED, global_group) self.parser = AzCliCommandParser(prog='az', parents=[self.global_parser]) self.raise_event(self.COMMAND_PARSER_CREATED, self.parser) @@ -134,9 +135,7 @@ def _enable_autocomplete(parser): argcomplete.autocomplete(parser) @staticmethod - def _register_builtin_arguments(parser): - global_group = parser.get_global_group() - + def _register_builtin_arguments(global_group): global_group.add_argument('--subscription', dest='_subscription_id', help=argparse.SUPPRESS) global_group.add_argument('--output', '-o', dest='_output_format', choices=['list', 'json', 'tsv'], diff --git a/src/azure/cli/extensions/query.py b/src/azure/cli/extensions/query.py index b38ecba26aa..a54579d22db 100644 --- a/src/azure/cli/extensions/query.py +++ b/src/azure/cli/extensions/query.py @@ -1,8 +1,7 @@ import collections -def _register_global_parameter(parser): +def _register_global_parameter(global_group): # Let the program know that we are adding a parameter --query - global_group = parser.get_global_group() global_group.add_argument('--query', dest='_jmespath_query', metavar='JMESPATH', help='JMESPath query string. See http://jmespath.org/ for more information and examples.') # pylint: disable=line-too-long diff --git a/src/azure/cli/parser.py b/src/azure/cli/parser.py index b92b4664d57..095107e2efd 100644 --- a/src/azure/cli/parser.py +++ b/src/azure/cli/parser.py @@ -17,11 +17,6 @@ def __init__(self, **kwargs): self.help_file = kwargs.pop('help_file', None) super(AzCliCommandParser, self).__init__(**kwargs) - def get_global_group(self): - if len(self._action_groups) < 3: - self.add_argument_group('global', 'global arguments') - return self._action_groups[2] - def load_command_table(self, command_table): """Load a command table into our parser. """ From 4c044539b5fd5da56b9792e1c4451f1829d279c1 Mon Sep 17 00:00:00 2001 From: Burt Bielicki Date: Mon, 2 May 2016 09:45:20 -0700 Subject: [PATCH 07/14] add global sort test --- src/azure/cli/_help.py | 3 +- src/azure/cli/tests/test_help.py | 54 ++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 1 deletion(-) diff --git a/src/azure/cli/_help.py b/src/azure/cli/_help.py index 3f6978ecba2..d22909cdce7 100644 --- a/src/azure/cli/_help.py +++ b/src/azure/cli/_help.py @@ -233,7 +233,8 @@ def _load_from_data(self, data): raise HelpAuthoringException('Extra help param {0}'.format(extra_param['name'])) self.parameters = loaded_params - def is_global(self, action): + @staticmethod + def is_global(action): return action.container.description == 'global arguments' \ or action.dest == 'help' diff --git a/src/azure/cli/tests/test_help.py b/src/azure/cli/tests/test_help.py index 08ad429b742..950c61670a5 100644 --- a/src/azure/cli/tests/test_help.py +++ b/src/azure/cli/tests/test_help.py @@ -533,6 +533,60 @@ def test_handler(args): ''' self.assertEqual(s, io.getvalue()) + @redirect_io + @mock.patch('azure.cli.application.Application.register', return_value=None) + @mock.patch('azure.cli.extensions.register_extensions', return_value=None) + def test_help_global_params(self, mock_register_extensions, _): + def register_globals(global_group): + global_group.add_argument('--query2', dest='_jmespath_query', metavar='JMESPATH', + help='JMESPath query string. See http://jmespath.org/ ' + 'for more information and examples.') + + mock_register_extensions.return_value = None + mock_register_extensions.side_effect = lambda app: \ + app._event_handlers[app.GLOBAL_PARSER_CREATED].append(register_globals) + + def test_handler(args): + pass + + cmd_table = { + test_handler: { + 'name': 'n1', + 'help_file': ''' + long-summary: | + line1 + line2 + ''', + 'arguments': [ + {'name': '--arg -a', 'required': False}, + {'name': '-b', 'required': False} + ] + } + } + config = Configuration([]) + config.get_command_table = lambda: cmd_table + app = Application(config) + + with self.assertRaises(SystemExit): + app.execute('n1 -h'.split()) + + s = """ +Command + az n1 + line1 + line2 + +Arguments + --arg -a + -b + +Global Arguments + --help -h: show this help message and exit + --query2 : JMESPath query string. See http://jmespath.org/ for more information and examples. +""" + + self.assertEqual(s, io.getvalue()) + if __name__ == '__main__': unittest.main() From fc99ddc5ec5fbf3936626e2a854afa99e64ff319 Mon Sep 17 00:00:00 2001 From: Burt Bielicki Date: Tue, 3 May 2016 09:56:40 -0700 Subject: [PATCH 08/14] print out all groups without special casing globals --- src/azure/cli/_help.py | 22 ++++++++++------------ src/azure/cli/application.py | 8 +++++--- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/azure/cli/_help.py b/src/azure/cli/_help.py index d22909cdce7..7756fdf6845 100644 --- a/src/azure/cli/_help.py +++ b/src/azure/cli/_help.py @@ -75,14 +75,16 @@ def print_arguments(help_file): required_tag = L(' [Required]') max_name_length = max(len(p.name) + (len(required_tag) if p.required else 0) for p in help_file.parameters) - added_global_space = False + last_group_name = None for p in sorted(help_file.parameters, - key=lambda p: str(not p.required) + str(p.global_param) + p.name): + key=lambda p: str(p.group_name or 'A') + str(not p.required) + p.name): indent = 1 required_text = required_tag if p.required else '' - if p.global_param and not added_global_space: - print('\nGlobal Arguments') - added_global_space = True + if p.group_name != last_group_name: + if p.group_name: + print('') + print(p.group_name) + last_group_name = p.group_name _print_indent('{0}{1}{2}{3}'.format(p.name, _get_column_indent(p.name + required_text, max_name_length), @@ -210,7 +212,7 @@ def __init__(self, delimiters, parser): self.parameters.append(HelpParameter(' '.join(sorted(action.option_strings)), action.help, required=action.required, - global_param=self.is_global(action))) + group_name=action.container.description)) def _load_from_data(self, data): super(CommandHelpFile, self)._load_from_data(data) @@ -233,20 +235,16 @@ def _load_from_data(self, data): raise HelpAuthoringException('Extra help param {0}'.format(extra_param['name'])) self.parameters = loaded_params - @staticmethod - def is_global(action): - return action.container.description == 'global arguments' \ - or action.dest == 'help' class HelpParameter(object): #pylint: disable=too-few-public-methods - def __init__(self, param_name, description, required, global_param=False): + def __init__(self, param_name, description, required, group_name=False): self.name = param_name self.required = required self.type = 'string' self.short_summary = description self.long_summary = '' self.value_sources = [] - self.global_param = global_param + self.group_name = group_name def update_from_data(self, data): if self.name != data.get('name'): diff --git a/src/azure/cli/application.py b/src/azure/cli/application.py index f410c91f413..502add0a342 100644 --- a/src/azure/cli/application.py +++ b/src/azure/cli/application.py @@ -50,7 +50,7 @@ def __init__(self, configuration): azure.cli.extensions.register_extensions(self) self.global_parser = AzCliCommandParser(prog='az', add_help=False) - global_group = self.global_parser.add_argument_group('global', 'global arguments') + global_group = self.global_parser.add_argument_group('global', 'Global Arguments') self.raise_event(self.GLOBAL_PARSER_CREATED, global_group) self.parser = AzCliCommandParser(prog='az', parents=[self.global_parser]) @@ -143,9 +143,11 @@ def _register_builtin_arguments(global_group): # The arguments for verbosity don't get parsed by argparse but we add it here for help. global_group.add_argument('--verbose', dest='_log_verbosity_verbose', help='Increase logging verbosity.' - ' Use --debug for full debug logs.') + ' Use --debug for full debug logs.', + action='store_true') global_group.add_argument('--debug', dest='_log_verbosity_debug', - help='Increase logging verbosity to show all debug logs.') + help='Increase logging verbosity to show all debug logs.', + action='store_true') def _handle_builtin_arguments(self, args): self.configuration.output_format = args._output_format #pylint: disable=protected-access From b7da0a16bc617c441ad4786be830c989609248f1 Mon Sep 17 00:00:00 2001 From: Burt Bielicki Date: Tue, 3 May 2016 11:16:53 -0700 Subject: [PATCH 09/14] special case help so it shows in global args --- src/azure/cli/_help.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/azure/cli/_help.py b/src/azure/cli/_help.py index 7756fdf6845..65587cc470f 100644 --- a/src/azure/cli/_help.py +++ b/src/azure/cli/_help.py @@ -77,7 +77,8 @@ def print_arguments(help_file): for p in help_file.parameters) last_group_name = None for p in sorted(help_file.parameters, - key=lambda p: str(p.group_name or 'A') + str(not p.required) + p.name): + key=lambda p: str(p.group_name or 'A') + + str(not p.required) + p.name): indent = 1 required_text = required_tag if p.required else '' if p.group_name != last_group_name: @@ -214,6 +215,9 @@ def __init__(self, delimiters, parser): required=action.required, group_name=action.container.description)) + help_param = next(p for p in self.parameters if p.name == '--help -h') + help_param.group_name = 'Global Arguments' + def _load_from_data(self, data): super(CommandHelpFile, self)._load_from_data(data) From d0b390ea62c14efc5e96bcfdaeb3d27e6386898f Mon Sep 17 00:00:00 2001 From: Burt Bielicki Date: Tue, 3 May 2016 13:11:38 -0700 Subject: [PATCH 10/14] enable remaining help tests --- src/azure/cli/tests/test_help.py | 130 +++++++++++++++---------------- 1 file changed, 65 insertions(+), 65 deletions(-) diff --git a/src/azure/cli/tests/test_help.py b/src/azure/cli/tests/test_help.py index 950c61670a5..1d18e593c2f 100644 --- a/src/azure/cli/tests/test_help.py +++ b/src/azure/cli/tests/test_help.py @@ -376,41 +376,42 @@ def test_handler(args): '.*Extra help param --foobar -fb.*', lambda: app.execute('n1 -h'.split())) -# Will uncomment when partial params don't bypass help (help behaviors implementation) task #115631559 -# @redirect_io -# def test_help_with_param_specified(self): -# app = Application(Configuration([])) -# def test_handler(args): -# pass - -# cmd_table = { -# test_handler: { -# 'name': 'n1', -# 'arguments': [ -# {'name': '--arg -a', 'required': False}, -# {'name': '-b', 'required': False} -# ] -# } -# } -# config = Configuration([]) - #config.get_command_table = lambda: cmd_table - #app = Application(config) - -# with self.assertRaises(SystemExit): -# cmd_result = app.execute('n1 --arg -h'.split()) - -# s = ''' -#Command -# n1 - -#Arguments -# --arg -a - -# -b - -#''' - -# self.assertEqual(s, io.getvalue()) + @redirect_io + @mock.patch('azure.cli.application.Application.register', return_value=None) + def test_help_with_param_specified(self, _): + app = Application(Configuration([])) + def test_handler(args): + pass + + cmd_table = { + test_handler: { + 'name': 'n1', + 'arguments': [ + {'name': '--arg -a', 'required': False}, + {'name': '-b', 'required': False} + ] + } + } + config = Configuration([]) + config.get_command_table = lambda: cmd_table + app = Application(config) + + with self.assertRaises(SystemExit): + cmd_result = app.execute('n1 --arg foo -h'.split()) + + s = ''' +Command + az n1 + +Arguments + --arg -a + -b + +Global Arguments + --help -h: show this help message and exit +''' + + self.assertEqual(s, io.getvalue()) @redirect_io def test_help_group_children(self): @@ -445,36 +446,35 @@ def test_handler2(args): s = '\nGroup\n az group1\n\nSub-Commands\n group2\n group3\n\n' self.assertEqual(s, io.getvalue()) - # Will uncomment when all errors are shown at once (help behaviors implementation) task #115631559 - #@redirect_io - #def test_help_extra_missing_params(self): - # app = Application(Configuration([])) - # def test_handler(args): - # pass - - # cmd_table = { - # test_handler: { - # 'name': 'n1', - # 'arguments': [ - # {'name': '--foobar -fb', 'required': False}, - # {'name': '--foobar2 -fb2', 'required': True} - # ] - # } - # } - # config = Configuration([]) - #config.get_command_table = lambda: cmd_table - #app = Application(config) - - # with self.assertRaises(SystemExit): - # app.execute('n1 -fb a --foobar3 bad'.split()) - - # with open(r'C:\temp\value.txt', 'w') as f: - # f.write(io.getvalue()) - - # self.assertTrue('required' in io.getvalue() - # and '--foobar/-fb' not in io.getvalue() - # and '--foobar2/-fb' in io.getvalue() - # and 'unrecognized arguments: --foobar3' in io.getvalue()) + @redirect_io + def test_help_extra_missing_params(self): + app = Application(Configuration([])) + def test_handler(args): + pass + + cmd_table = { + test_handler: { + 'name': 'n1', + 'arguments': [ + {'name': '--foobar -fb', 'required': False}, + {'name': '--foobar2 -fb2', 'required': True} + ] + } + } + config = Configuration([]) + config.get_command_table = lambda: cmd_table + app = Application(config) + + with self.assertRaises(SystemExit): + app.execute('n1 -fb a --foobar value'.split()) + + with self.assertRaises(SystemExit): + app.execute('n1 -fb a --foobar2 value --foobar3 extra'.split()) + + self.assertTrue('required' in io.getvalue() + and '--foobar/-fb' not in io.getvalue() + and '--foobar2/-fb' in io.getvalue() + and 'unrecognized arguments: --foobar3' in io.getvalue()) @redirect_io def test_help_group_help(self): From 74585227759be3978e497480b4d2befcab92bedc Mon Sep 17 00:00:00 2001 From: Burt Bielicki Date: Wed, 4 May 2016 11:33:39 -0700 Subject: [PATCH 11/14] update tests after merge master --- src/azure/cli/tests/test_help.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/azure/cli/tests/test_help.py b/src/azure/cli/tests/test_help.py index 67677ba4af7..2267d563b45 100644 --- a/src/azure/cli/tests/test_help.py +++ b/src/azure/cli/tests/test_help.py @@ -232,11 +232,11 @@ def test_handler(args): Arguments --foobar2 -fb2 [Required]: one line partial sentence - paragraph(s) + paragraph(s) --foobar -fb : one line partial sentence - text, markdown, etc. - Values from: az vm list, default + text, markdown, etc. + Values from: az vm list, default --foobar3 -fb3 : the foobar3 @@ -298,11 +298,12 @@ def test_handler(args): Arguments --foobar2 -fb2 [Required]: one line partial sentence - paragraph(s) + paragraph(s) --foobar -fb : one line partial sentence - text, markdown, etc. - Values from: az vm list, default + text, markdown, etc. + Values from: az vm list, default + Global Arguments --help -h : show this help message and exit From 11cf57b18f5e6cd7b356959df210bbd141c6bc72 Mon Sep 17 00:00:00 2001 From: Burt Bielicki Date: Wed, 4 May 2016 15:12:06 -0700 Subject: [PATCH 12/14] work around argparse bug --- src/azure/cli/tests/test_help.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/azure/cli/tests/test_help.py b/src/azure/cli/tests/test_help.py index 2267d563b45..263bb255688 100644 --- a/src/azure/cli/tests/test_help.py +++ b/src/azure/cli/tests/test_help.py @@ -469,8 +469,12 @@ def test_handler(args): config.get_command_table = lambda: cmd_table app = Application(config) - with self.assertRaises(SystemExit): + # there is an argparse bug on <2.7.10 where SystemExit is not thrown on missing required param + if sys.version_info < (2, 7, 10): app.execute('n1 -fb a --foobar value'.split()) + else: + with self.assertRaises(SystemExit): + app.execute('n1 -fb a --foobar value'.split()) with self.assertRaises(SystemExit): app.execute('n1 -fb a --foobar2 value --foobar3 extra'.split()) From 1a407f6507c4c0fa3f2f93fb2c7080c8d23f48a9 Mon Sep 17 00:00:00 2001 From: Burt Bielicki Date: Wed, 4 May 2016 15:29:34 -0700 Subject: [PATCH 13/14] more workaround --- src/azure/cli/tests/test_help.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/azure/cli/tests/test_help.py b/src/azure/cli/tests/test_help.py index 263bb255688..feeb41a474c 100644 --- a/src/azure/cli/tests/test_help.py +++ b/src/azure/cli/tests/test_help.py @@ -472,12 +472,12 @@ def test_handler(args): # there is an argparse bug on <2.7.10 where SystemExit is not thrown on missing required param if sys.version_info < (2, 7, 10): app.execute('n1 -fb a --foobar value'.split()) + app.execute('n1 -fb a --foobar2 value --foobar3 extra'.split()) else: with self.assertRaises(SystemExit): app.execute('n1 -fb a --foobar value'.split()) - - with self.assertRaises(SystemExit): - app.execute('n1 -fb a --foobar2 value --foobar3 extra'.split()) + with self.assertRaises(SystemExit): + app.execute('n1 -fb a --foobar2 value --foobar3 extra'.split()) self.assertTrue('required' in io.getvalue() and '--foobar/-fb' not in io.getvalue() From f25ba60f5ce94c9b28ded3e290cce65238661585 Mon Sep 17 00:00:00 2001 From: Burt Bielicki Date: Wed, 4 May 2016 16:23:25 -0700 Subject: [PATCH 14/14] workaround --- src/azure/cli/tests/test_help.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/azure/cli/tests/test_help.py b/src/azure/cli/tests/test_help.py index feeb41a474c..1cbbd37c58d 100644 --- a/src/azure/cli/tests/test_help.py +++ b/src/azure/cli/tests/test_help.py @@ -479,10 +479,10 @@ def test_handler(args): with self.assertRaises(SystemExit): app.execute('n1 -fb a --foobar2 value --foobar3 extra'.split()) - self.assertTrue('required' in io.getvalue() - and '--foobar/-fb' not in io.getvalue() - and '--foobar2/-fb' in io.getvalue() - and 'unrecognized arguments: --foobar3' in io.getvalue()) + self.assertTrue('required' in io.getvalue() + and '--foobar/-fb' not in io.getvalue() + and '--foobar2/-fb2' in io.getvalue() + and 'unrecognized arguments: --foobar3 extra' in io.getvalue()) @redirect_io def test_help_group_help(self):