From fa093d8adf03c88908caa38fe70e0db2711e801c Mon Sep 17 00:00:00 2001 From: Felix Fontein Date: Thu, 8 Sep 2022 22:02:58 +0200 Subject: [PATCH] ansible-doc: remove manual formatting (#78668) * remove manual formatting and make the output align closer with the original YAML Co-authored-by: Brian Coca --- .../78668-ansible-doc-formatting.yml | 2 + lib/ansible/cli/doc.py | 64 ++++++++----------- .../targets/ansible-doc/fakecollrole.output | 1 - .../targets/ansible-doc/fakemodule.output | 2 +- .../targets/ansible-doc/fakerole.output | 6 +- .../ansible-doc/randommodule-text.output | 18 ++---- .../ansible-doc/test_docs_returns.output | 5 +- .../ansible-doc/test_docs_suboptions.output | 10 +-- .../ansible-doc/test_docs_yaml_anchors.output | 10 ++- 9 files changed, 51 insertions(+), 67 deletions(-) create mode 100644 changelogs/fragments/78668-ansible-doc-formatting.yml diff --git a/changelogs/fragments/78668-ansible-doc-formatting.yml b/changelogs/fragments/78668-ansible-doc-formatting.yml new file mode 100644 index 00000000000..937deaeec62 --- /dev/null +++ b/changelogs/fragments/78668-ansible-doc-formatting.yml @@ -0,0 +1,2 @@ +minor_changes: + - "ansible-doc - remove some of the manual formatting, and use YAML more uniformly. This in particular means that ``true`` and ``false`` are used for boolean values, instead of ``True`` and ``False`` (https://github.com/ansible/ansible/pull/78668)." diff --git a/lib/ansible/cli/doc.py b/lib/ansible/cli/doc.py index b2e2ed9aa42..48292c26b1e 100755 --- a/lib/ansible/cli/doc.py +++ b/lib/ansible/cli/doc.py @@ -19,7 +19,6 @@ import traceback import ansible.plugins.loader as plugin_loader -from collections.abc import Sequence from pathlib import Path from ansible import constants as C @@ -28,6 +27,7 @@ from ansible.cli.arguments import option_helpers as opt_help from ansible.collections.list import list_collection_dirs from ansible.errors import AnsibleError, AnsibleOptionsError, AnsibleParserError, AnsiblePluginNotFound from ansible.module_utils._text import to_native, to_text +from ansible.module_utils.common.collections import is_sequence from ansible.module_utils.common.json import json_dump from ansible.module_utils.common.yaml import yaml_dump from ansible.module_utils.compat import importlib @@ -835,7 +835,7 @@ class DocCLI(CLI, RoleMixin): self._display_role_doc(docs) elif docs: - text = DocCLI._dump_yaml(docs, '') + text = DocCLI.tty_ify(DocCLI._dump_yaml(docs)) if text: DocCLI.pager(''.join(text)) @@ -1000,8 +1000,12 @@ class DocCLI(CLI, RoleMixin): return os.pathsep.join(ret) @staticmethod - def _dump_yaml(struct, indent): - return DocCLI.tty_ify('\n'.join([indent + line for line in yaml_dump(struct, default_flow_style=False, Dumper=AnsibleDumper).split('\n')])) + def _dump_yaml(struct, flow_style=False): + return yaml_dump(struct, default_flow_style=flow_style, default_style="''", Dumper=AnsibleDumper).rstrip('\n') + + @staticmethod + def _indent_lines(text, indent): + return DocCLI.tty_ify('\n'.join([indent + line for line in text.split('\n')])) @staticmethod def _format_version_added(version_added, version_added_collection=None): @@ -1021,6 +1025,7 @@ class DocCLI(CLI, RoleMixin): # Create a copy so we don't modify the original (in case YAML anchors have been used) opt = dict(fields[o]) + # required is used as indicator and removed required = opt.pop('required', False) if not isinstance(required, bool): raise AnsibleError("Incorrect value for 'Required', a boolean is needed.: %s" % required) @@ -1031,9 +1036,10 @@ class DocCLI(CLI, RoleMixin): text.append("%s%s %s" % (base_indent, opt_leadin, o)) + # description is specifically formated and can either be string or list of strings if 'description' not in opt: raise AnsibleError("All (sub-)options and return values must have a 'description' field") - if isinstance(opt['description'], list): + if is_sequence(opt['description']): for entry_idx, entry in enumerate(opt['description'], 1): if not isinstance(entry, string_types): raise AnsibleError("Expected string in description of %s at index %s, got %s" % (o, entry_idx, type(entry))) @@ -1044,29 +1050,15 @@ class DocCLI(CLI, RoleMixin): text.append(textwrap.fill(DocCLI.tty_ify(opt['description']), limit, initial_indent=opt_indent, subsequent_indent=opt_indent)) del opt['description'] - aliases = '' - if 'aliases' in opt: - if len(opt['aliases']) > 0: - aliases = "(Aliases: " + ", ".join(to_text(i) for i in opt['aliases']) + ")" - del opt['aliases'] - choices = '' - if 'choices' in opt: - if len(opt['choices']) > 0: - choices = "(Choices: " + ", ".join(to_text(i) for i in opt['choices']) + ")" - del opt['choices'] - default = '' - if not return_values: - if 'default' in opt or not required: - default = "[Default: %s" % to_text(opt.pop('default', '(null)')) + "]" - - text.append(textwrap.fill(DocCLI.tty_ify(aliases + choices + default), limit, - initial_indent=opt_indent, subsequent_indent=opt_indent)) - suboptions = [] for subkey in ('options', 'suboptions', 'contains', 'spec'): if subkey in opt: suboptions.append((subkey, opt.pop(subkey))) + if not required and not return_values and 'default' not in opt: + opt['default'] = None + + # sanitize config items conf = {} for config in ('env', 'ini', 'yaml', 'vars', 'keyword'): if config in opt and opt[config]: @@ -1077,6 +1069,7 @@ class DocCLI(CLI, RoleMixin): if ignore in item: del item[ignore] + # reformat cli optoins if 'cli' in opt and opt['cli']: conf['cli'] = [] for cli in opt['cli']: @@ -1086,24 +1079,23 @@ class DocCLI(CLI, RoleMixin): conf['cli'].append(cli) del opt['cli'] + # add custom header for conf if conf: - text.append(DocCLI._dump_yaml({'set_via': conf}, opt_indent)) + text.append(DocCLI._indent_lines(DocCLI._dump_yaml({'set_via': conf}), opt_indent)) + # these we handle at the end of generic option processing version_added = opt.pop('version_added', None) version_added_collection = opt.pop('version_added_collection', None) + # general processing for options for k in sorted(opt): if k.startswith('_'): continue - if isinstance(opt[k], string_types): - text.append('%s%s: %s' % (opt_indent, k, - textwrap.fill(DocCLI.tty_ify(opt[k]), - limit - (len(k) + 2), - subsequent_indent=opt_indent))) - elif isinstance(opt[k], (Sequence)) and all(isinstance(x, string_types) for x in opt[k]): - text.append(DocCLI.tty_ify('%s%s: %s' % (opt_indent, k, ', '.join(opt[k])))) + + if is_sequence(opt[k]): + text.append(DocCLI._indent_lines('%s: %s' % (k, DocCLI._dump_yaml(opt[k], flow_style=True)), opt_indent)) else: - text.append(DocCLI._dump_yaml({k: opt[k]}, opt_indent)) + text.append(DocCLI._indent_lines(DocCLI._dump_yaml({k: opt[k]}), opt_indent)) if version_added: text.append("%sadded in: %s\n" % (opt_indent, DocCLI._format_version_added(version_added, version_added_collection))) @@ -1157,7 +1149,7 @@ class DocCLI(CLI, RoleMixin): if doc.get('attributes'): text.append("ATTRIBUTES:\n") - text.append(DocCLI._dump_yaml(doc.pop('attributes'), opt_indent)) + text.append(DocCLI._indent_lines(DocCLI._dump_yaml(doc.pop('attributes')), opt_indent)) text.append('') # generic elements we will handle identically @@ -1171,7 +1163,7 @@ class DocCLI(CLI, RoleMixin): text.append('%s: %s' % (k.upper(), ', '.join(doc[k]))) else: # use empty indent since this affects the start of the yaml doc, not it's keys - text.append(DocCLI._dump_yaml({k.upper(): doc[k]}, '')) + text.append(DocCLI._indent_lines(DocCLI._dump_yaml({k.upper(): doc[k]}), '')) text.append('') return text @@ -1231,7 +1223,7 @@ class DocCLI(CLI, RoleMixin): if doc.get('attributes', False): text.append("ATTRIBUTES:\n") - text.append(DocCLI._dump_yaml(doc.pop('attributes'), opt_indent)) + text.append(DocCLI._indent_lines(DocCLI._dump_yaml(doc.pop('attributes')), opt_indent)) text.append('') if doc.get('notes', False): @@ -1286,7 +1278,7 @@ class DocCLI(CLI, RoleMixin): text.append('%s: %s' % (k.upper(), ', '.join(doc[k]))) else: # use empty indent since this affects the start of the yaml doc, not it's keys - text.append(DocCLI._dump_yaml({k.upper(): doc[k]}, '')) + text.append(DocCLI._indent_lines(DocCLI._dump_yaml({k.upper(): doc[k]}), '')) del doc[k] text.append('') diff --git a/test/integration/targets/ansible-doc/fakecollrole.output b/test/integration/targets/ansible-doc/fakecollrole.output index fdd6a2dda63..3ae9077f884 100644 --- a/test/integration/targets/ansible-doc/fakecollrole.output +++ b/test/integration/targets/ansible-doc/fakecollrole.output @@ -9,7 +9,6 @@ OPTIONS (= is mandatory): = altopt1 altopt1 description - type: int diff --git a/test/integration/targets/ansible-doc/fakemodule.output b/test/integration/targets/ansible-doc/fakemodule.output index 5548ad5ecef..4fb0776f345 100644 --- a/test/integration/targets/ansible-doc/fakemodule.output +++ b/test/integration/targets/ansible-doc/fakemodule.output @@ -8,7 +8,7 @@ OPTIONS (= is mandatory): - _notreal really not a real option - [Default: (null)] + default: null AUTHOR: me diff --git a/test/integration/targets/ansible-doc/fakerole.output b/test/integration/targets/ansible-doc/fakerole.output index 81db9a63fe8..bcb53dccce9 100644 --- a/test/integration/targets/ansible-doc/fakerole.output +++ b/test/integration/targets/ansible-doc/fakerole.output @@ -15,17 +15,17 @@ OPTIONS (= is mandatory): = myopt1 First option. - type: str - myopt2 Second option - [Default: 8000] + default: 8000 type: int - myopt3 Third option. - (Choices: choice1, choice2)[Default: (null)] + choices: [choice1, choice2] + default: null type: str diff --git a/test/integration/targets/ansible-doc/randommodule-text.output b/test/integration/targets/ansible-doc/randommodule-text.output index 24327a59552..51d7930a079 100644 --- a/test/integration/targets/ansible-doc/randommodule-text.output +++ b/test/integration/targets/ansible-doc/randommodule-text.output @@ -15,7 +15,6 @@ OPTIONS (= is mandatory): - sub Suboptions. - [Default: (null)] set_via: env: - deprecated: @@ -24,14 +23,14 @@ OPTIONS (= is mandatory): version: 2.0.0 why: Test deprecation name: TEST_ENV - + default: null type: dict OPTIONS: - subtest2 Another suboption. - [Default: (null)] + default: null type: float added in: version 1.1.0 @@ -41,28 +40,28 @@ OPTIONS (= is mandatory): - subtest A suboption. - [Default: (null)] + default: null type: int added in: version 1.1.0 of testns.testcol - test Some text. - [Default: (null)] + default: null type: str added in: version 1.2.0 of testns.testcol - testcol2option An option taken from testcol2 - [Default: (null)] + default: null type: str added in: version 1.0.0 of testns.testcol2 - testcol2option2 Another option taken from testcol2 - [Default: (null)] + default: null type: str @@ -76,14 +75,12 @@ EXAMPLES: RETURN VALUES: - a_first A first result. - returned: success type: str - m_middle This should be in the middle. Has some more data - returned: success and 1st of month type: dict @@ -91,14 +88,13 @@ RETURN VALUES: - suboption A suboption. - (Choices: ARF, BARN, c_without_capital_first_letter) + choices: [ARF, BARN, c_without_capital_first_letter] type: str added in: version 1.4.0 of testns.testcol - z_last A last result. - returned: success type: str added in: version 1.3.0 of testns.testcol diff --git a/test/integration/targets/ansible-doc/test_docs_returns.output b/test/integration/targets/ansible-doc/test_docs_returns.output index 3730a90977c..3e2364570b0 100644 --- a/test/integration/targets/ansible-doc/test_docs_returns.output +++ b/test/integration/targets/ansible-doc/test_docs_returns.output @@ -11,14 +11,12 @@ EXAMPLES: RETURN VALUES: - a_first A first result. - returned: success type: str - m_middle This should be in the middle. Has some more data - returned: success and 1st of month type: dict @@ -26,11 +24,10 @@ RETURN VALUES: - suboption A suboption. - (Choices: ARF, BARN, c_without_capital_first_letter) + choices: [ARF, BARN, c_without_capital_first_letter] type: str - z_last A last result. - returned: success type: str diff --git a/test/integration/targets/ansible-doc/test_docs_suboptions.output b/test/integration/targets/ansible-doc/test_docs_suboptions.output index 3092e267705..350f90f1bde 100644 --- a/test/integration/targets/ansible-doc/test_docs_suboptions.output +++ b/test/integration/targets/ansible-doc/test_docs_suboptions.output @@ -6,31 +6,31 @@ OPTIONS (= is mandatory): - with_suboptions An option with suboptions. Use with care. - [Default: (null)] + default: null type: dict SUBOPTIONS: - a_first The first suboption. - [Default: (null)] + default: null type: str - m_middle The suboption in the middle. Has its own suboptions. - [Default: (null)] + default: null SUBOPTIONS: - a_suboption A sub-suboption. - [Default: (null)] + default: null type: str - z_last The last suboption. - [Default: (null)] + default: null type: str diff --git a/test/integration/targets/ansible-doc/test_docs_yaml_anchors.output b/test/integration/targets/ansible-doc/test_docs_yaml_anchors.output index 8d49e121125..5eb2eee2be3 100644 --- a/test/integration/targets/ansible-doc/test_docs_yaml_anchors.output +++ b/test/integration/targets/ansible-doc/test_docs_yaml_anchors.output @@ -5,12 +5,12 @@ OPTIONS (= is mandatory): - at_the_top Short desc - [Default: some string] + default: some string type: str - egress Egress firewall rules - [Default: (null)] + default: null elements: dict type: list @@ -18,12 +18,11 @@ OPTIONS (= is mandatory): = port Rule port - type: int - ingress Ingress firewall rules - [Default: (null)] + default: null elements: dict type: list @@ -31,12 +30,11 @@ OPTIONS (= is mandatory): = port Rule port - type: int - last_one Short desc - [Default: some string] + default: some string type: str