Peter Maydell <peter.mayd...@linaro.org> writes:
> Make the handling of indentation in doc comments more sophisticated,
> so that when we see a section like:
>
> Notes: some text
> some more text
> indented line 3
>
> we save it for the doc-comment processing code as:
>
> some text
> some more text
> indented line 3
>
> and when we see a section with the heading on its own line:
>
> Notes:
>
> some text
> some more text
> indented text
>
> we also accept that and save it in the same form.
>
> If we detect that the comment document text is not indented as much
> as we expect it to be, we throw a parse error. (We don't complain
> about over-indented sections, because for rST this can be legitimate
> markup.)
>
> The golden reference for the doc comment text is updated to remove
> the two 'wrong' indents; these now form a test case that we correctly
> stripped leading whitespace from an indented multi-line argument
> definition.
>
> We update the documentation in docs/devel/qapi-code-gen.txt to
> describe the new indentation rules.
>
> Signed-off-by: Peter Maydell <peter.mayd...@linaro.org>
> ---
> docs/devel/qapi-code-gen.txt | 23 +++++++
> scripts/qapi/parser.py | 93 +++++++++++++++++++++------
> tests/qapi-schema/doc-bad-indent.err | 1 +
> tests/qapi-schema/doc-bad-indent.json | 8 +++
> tests/qapi-schema/doc-bad-indent.out | 0
> tests/qapi-schema/doc-good.out | 4 +-
> tests/qapi-schema/meson.build | 1 +
> 7 files changed, 109 insertions(+), 21 deletions(-)
> create mode 100644 tests/qapi-schema/doc-bad-indent.err
> create mode 100644 tests/qapi-schema/doc-bad-indent.json
> create mode 100644 tests/qapi-schema/doc-bad-indent.out
>
> diff --git a/docs/devel/qapi-code-gen.txt b/docs/devel/qapi-code-gen.txt
> index 9eede44350c..69eaffac376 100644
> --- a/docs/devel/qapi-code-gen.txt
> +++ b/docs/devel/qapi-code-gen.txt
> @@ -901,6 +901,22 @@ commands and events), member (for structs and unions),
> branch (for
> alternates), or value (for enums), and finally optional tagged
> sections.
>
> +Descriptions of arguments can span multiple lines. The description
> +text can start on the line following the '@argname:', in which case
> +it must not be indented at all. It can also start on the same line
> +as the '@argname:'. In this case if it spans multiple lines then
> +second and subsequent lines must be indented to line up with the
> +first character of the first line of the description:
Please put two spaces after sentence-ending punctuation, for local
consistency, and to keep Emacs sentence commands working.
Can touch this up in my tree, of course.
> +
> +# @argone:
> +# This is a two line description
> +# in the first style.
> +#
> +# @argtwo: This is a two line description
> +# in the second style.
> +
> +The number of spaces between the ':' and the text is not significant.
> +
> FIXME: the parser accepts these things in almost any order.
> FIXME: union branches should be described, too.
>
> @@ -911,6 +927,13 @@ A tagged section starts with one of the following words:
> "Note:"/"Notes:", "Since:", "Example"/"Examples", "Returns:", "TODO:".
> The section ends with the start of a new section.
>
> +The text of a section can start on a new line, in
> +which case it must not be indented at all. It can also start
> +on the same line as the 'Note:', 'Returns:', etc tag. In this
> +case if it spans multiple lines then second and subsequent
> +lines must be indented to match the first, in the same way as
> +multiline argument descriptions.
> +
> A 'Since: x.y.z' tagged section lists the release that introduced the
> definition.
>
> diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py
> index 04bf10db378..6c3455b41f3 100644
> --- a/scripts/qapi/parser.py
> +++ b/scripts/qapi/parser.py
> @@ -319,17 +319,32 @@ class QAPIDoc:
> """
>
> class Section:
> - def __init__(self, name=None):
> + def __init__(self, parser, name=None, indent=0):
> + # parser, for error messages about indentation
> + self._parser = parser
> # optional section name (argument/member or section name)
> self.name = name
> self.text = ''
> + # the expected indent level of the text of this section
> + self._indent = indent
>
> def append(self, line):
> + # Strip leading spaces corresponding to the expected indent level
> + # Blank lines are always OK.
> + if line:
> + indent = re.match(r'\s*', line).end()
> + if indent < self._indent:
> + raise QAPIParseError(
> + self._parser,
> + "unexpected de-indent (expected at least %d spaces)"
> %
> + self._indent)
> + line = line[self._indent:]
> +
> self.text += line.rstrip() + '\n'
>
> class ArgSection(Section):
> - def __init__(self, name):
> - super().__init__(name)
> + def __init__(self, parser, name, indent=0):
> + super().__init__(parser, name, indent)
> self.member = None
>
> def connect(self, member):
> @@ -343,7 +358,7 @@ class QAPIDoc:
> self._parser = parser
> self.info = info
> self.symbol = None
> - self.body = QAPIDoc.Section()
> + self.body = QAPIDoc.Section(parser)
> # dict mapping parameter name to ArgSection
> self.args = OrderedDict()
> self.features = OrderedDict()
> @@ -447,8 +462,21 @@ class QAPIDoc:
> name = line.split(' ', 1)[0]
>
> if name.startswith('@') and name.endswith(':'):
> - line = line[len(name)+1:]
> - self._start_args_section(name[1:-1])
> + # If line is "@arg: first line of description", find the
> + # index of 'f', which is the indent we expect for any
> + # following lines. We then remove the leading "@arg:" from line
PEP 8: You should use two spaces after a sentence-ending period in
multi-sentence comments, except after the final sentence.
More of the same below. Can touch it up in my tree.
> + # and replace it with spaces so that 'f' has the same index
> + # as it did in the original line and can be handled the same
> + # way we will handle following lines.
> + indent = re.match(r'@\S*:\s*', line).end()
> + line = line[indent:]
> + if not line:
> + # Line was just the "@arg:" header; following lines
> + # are not indented
> + indent = 0
> + else:
> + line = ' ' * indent + line
> + self._start_args_section(name[1:-1], indent)
> elif self._is_section_tag(name):
> self._append_line = self._append_various_line
> self._append_various_line(line)
> @@ -469,8 +497,21 @@ class QAPIDoc:
> name = line.split(' ', 1)[0]
>
> if name.startswith('@') and name.endswith(':'):
> - line = line[len(name)+1:]
> - self._start_features_section(name[1:-1])
> + # If line is "@arg: first line of description", find the
> + # index of 'f', which is the indent we expect for any
> + # following lines. We then remove the leading "@arg:" from line
> + # and replace it with spaces so that 'f' has the same index
> + # as it did in the original line and can be handled the same
> + # way we will handle following lines.
> + indent = re.match(r'@\S*:\s*', line).end()
> + line = line[indent:]
> + if not line:
> + # Line was just the "@arg:" header; following lines
> + # are not indented
> + indent = 0
> + else:
> + line = ' ' * indent + line
> + self._start_features_section(name[1:-1], indent)
> elif self._is_section_tag(name):
> self._append_line = self._append_various_line
> self._append_various_line(line)
> @@ -502,12 +543,25 @@ class QAPIDoc:
> "'%s' can't follow '%s' section"
> % (name, self.sections[0].name))
> if self._is_section_tag(name):
> - line = line[len(name)+1:]
> - self._start_section(name[:-1])
> + # If line is "Section: first line of description", find the
> + # index of 'f', which is the indent we expect for any
> + # following lines. We then remove the leading "Section:" from
> line
> + # and replace it with spaces so that 'f' has the same index
> + # as it did in the original line and can be handled the same
> + # way we will handle following lines.
> + indent = re.match(r'\S*:\s*', line).end()
> + line = line[indent:]
> + if not line:
> + # Line was just the "Section:" header; following lines
> + # are not indented
> + indent = 0
> + else:
> + line = ' ' * indent + line
We may want to de-triplicate. Not today.
> + self._start_section(name[:-1], indent)
>
> self._append_freeform(line)
>
> - def _start_symbol_section(self, symbols_dict, name):
> + def _start_symbol_section(self, symbols_dict, name, indent):
> # FIXME invalid names other than the empty string aren't flagged
> if not name:
> raise QAPIParseError(self._parser, "invalid parameter name")
> @@ -516,21 +570,21 @@ class QAPIDoc:
> "'%s' parameter name duplicated" % name)
> assert not self.sections
> self._end_section()
> - self._section = QAPIDoc.ArgSection(name)
> + self._section = QAPIDoc.ArgSection(self._parser, name, indent)
> symbols_dict[name] = self._section
>
> - def _start_args_section(self, name):
> - self._start_symbol_section(self.args, name)
> + def _start_args_section(self, name, indent):
> + self._start_symbol_section(self.args, name, indent)
>
> - def _start_features_section(self, name):
> - self._start_symbol_section(self.features, name)
> + def _start_features_section(self, name, indent):
> + self._start_symbol_section(self.features, name, indent)
>
> - def _start_section(self, name=None):
> + def _start_section(self, name=None, indent=0):
> if name in ('Returns', 'Since') and self.has_section(name):
> raise QAPIParseError(self._parser,
> "duplicated '%s' section" % name)
> self._end_section()
> - self._section = QAPIDoc.Section(name)
> + self._section = QAPIDoc.Section(self._parser, name, indent)
> self.sections.append(self._section)
>
> def _end_section(self):
> @@ -553,7 +607,8 @@ class QAPIDoc:
> def connect_member(self, member):
> if member.name not in self.args:
> # Undocumented TODO outlaw
> - self.args[member.name] = QAPIDoc.ArgSection(member.name)
> + self.args[member.name] = QAPIDoc.ArgSection(self._parser,
> + member.name)
> self.args[member.name].connect(member)
>
> def connect_feature(self, feature):
> diff --git a/tests/qapi-schema/doc-bad-indent.err
> b/tests/qapi-schema/doc-bad-indent.err
> new file mode 100644
> index 00000000000..67844539bd2
> --- /dev/null
> +++ b/tests/qapi-schema/doc-bad-indent.err
> @@ -0,0 +1 @@
> +doc-bad-indent.json:6:1: unexpected de-indent (expected at least 4 spaces)
> diff --git a/tests/qapi-schema/doc-bad-indent.json
> b/tests/qapi-schema/doc-bad-indent.json
> new file mode 100644
> index 00000000000..edde8f21dc7
> --- /dev/null
> +++ b/tests/qapi-schema/doc-bad-indent.json
> @@ -0,0 +1,8 @@
> +# Multiline doc comments should have consistent indentation
> +
> +##
> +# @foo:
> +# @a: line one
> +# line two is wrongly indented
> +##
> +{ 'command': 'foo', 'data': { 'a': 'int' } }
> diff --git a/tests/qapi-schema/doc-bad-indent.out
> b/tests/qapi-schema/doc-bad-indent.out
> new file mode 100644
> index 00000000000..e69de29bb2d
> diff --git a/tests/qapi-schema/doc-good.out b/tests/qapi-schema/doc-good.out
> index 9993ffcd89d..b7e3f4313da 100644
> --- a/tests/qapi-schema/doc-good.out
> +++ b/tests/qapi-schema/doc-good.out
> @@ -159,7 +159,7 @@ doc symbol=Alternate
>
> arg=i
> an integer
> - @b is undocumented
> +@b is undocumented
> arg=b
>
> feature=alt-feat
> @@ -174,7 +174,7 @@ doc symbol=cmd
> the first argument
> arg=arg2
> the second
> - argument
> +argument
> arg=arg3
>
> feature=cmd-feat1
> diff --git a/tests/qapi-schema/meson.build b/tests/qapi-schema/meson.build
> index f1449298b07..83a0a68389b 100644
> --- a/tests/qapi-schema/meson.build
> +++ b/tests/qapi-schema/meson.build
> @@ -53,6 +53,7 @@ schemas = [
> 'doc-bad-enum-member.json',
> 'doc-bad-event-arg.json',
> 'doc-bad-feature.json',
> + 'doc-bad-indent.json',
> 'doc-bad-section.json',
> 'doc-bad-symbol.json',
> 'doc-bad-union-member.json',
With trivial whitespace touch-ups:
Reviewed-by: Markus Armbruster <arm...@redhat.com>
