Kevin Wolf <kw...@redhat.com> writes:
> Documentation comments follow a certain structure: First, we have a text
> with a general description (called QAPIDoc.body). After this,
> descriptions of the arguments follow. Finally, we have a part that
> contains various named sections.
>
> The code doesn't show this structure, but just checks various attributes
> that indicate indirectly which part is being processed, so it happens to
> do the right set of things in the right phase. This is hard to follow,
> and adding support for documentation of features would be even harder.
>
> This patch restructures the code so that the three parts are clearly
> separated. The code becomes a bit longer, but easier to follow. The
> resulting output remains unchanged.
>
> Signed-off-by: Kevin Wolf <kw...@redhat.com>
> ---
> scripts/qapi/common.py | 122 ++++++++++++++++++++++++++++++++---------
> 1 file changed, 97 insertions(+), 25 deletions(-)
>
> diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
> index 9e4b6c00b5..55ccd216cf 100644
> --- a/scripts/qapi/common.py
> +++ b/scripts/qapi/common.py
> @@ -120,6 +120,22 @@ class QAPIDoc(object):
> def connect(self, member):
> self.member = member
>
> + class DocPart:
> + """
Did you drop the "A documentation part" headline intentionally?
> + Expression documentation blocks consist of
> + * a BODY part: first line naming the expression, plus an
> + optional overview
> + * an ARGS part: description of each argument (for commands and
> + events) or member (for structs, unions and alternates),
> + * a VARIOUS part: optional tagged sections.
> +
> + Free-form documentation blocks consist only of a BODY part.
> + """
> + # TODO Make it a subclass of Enum when Python 2 support is removed
> + BODY = 1
> + ARGS = 2
> + VARIOUS = 3
> +
> def __init__(self, parser, info):
> # self._parser is used to report errors with QAPIParseError. The
> # resulting error position depends on the state of the parser.
> @@ -135,6 +151,7 @@ class QAPIDoc(object):
> self.sections = []
> # the current section
> self._section = self.body
> + self._part = QAPIDoc.DocPart.BODY
>
> def has_section(self, name):
> """Return True if we have a section with this name."""
> @@ -144,7 +161,24 @@ class QAPIDoc(object):
> return False
>
> def append(self, line):
> - """Parse a comment line and add it to the documentation."""
> + """
> + Parse a comment line and add it to the documentation.
> +
> + The way that the line is dealt with depends on which part of the
> + documentation we're parsing right now:
> +
> + BODY means that we're ready to process free-form text into
> self.body. A
> + symbol name is only allowed if no other text was parsed yet. It is
> + interpreted as the symbol name that describes the currently
> documented
> + object. On getting the second symbol name, we proceed to ARGS.
> +
> + ARGS means that we're parsing the arguments section. Any symbol name
> is
> + interpreted as an argument and an ArgSection is created for it.
> +
> + VARIOUS is the final part where free-form sections may appear. This
> + includes named sections such as "Return:" as well as unnamed
> + paragraphs. Symbols are not allowed any more in this part.
> + """
A few little things:
* The reader has to make the connection from BODY, ARGS, VARIOUS to
self._part. To help him a bit, I'd say something like "depends on
which part of the documentation we're parsing right now, according to
self._part:"
* In the paragraph on BODY: on first reading, "A symbol name is only
allowed if no other text was parsed yet" appears to contradict "On
getting the second symbol name". It doesn't: the second symbol name
doesn't belong to this part. Perhaps we could phrase this more
clearly. Not sure it's worth the trouble.
* PEP 8: "For flowing long blocks of text with fewer structural
restrictions (docstrings or comments), the line length should be
limited to 72 characters."
* PEP 8: "You should use two spaces after a sentence-ending period in
multi-sentence comments, except after the final sentence."
> line = line[1:]
> if not line:
> self._append_freeform(line)
> @@ -154,37 +188,85 @@ class QAPIDoc(object):
> raise QAPIParseError(self._parser, "Missing space after #")
> line = line[1:]
>
> + if self._part == QAPIDoc.DocPart.BODY:
> + self._append_body_line(line)
> + elif self._part == QAPIDoc.DocPart.ARGS:
> + self._append_args_line(line)
> + elif self._part == QAPIDoc.DocPart.VARIOUS:
> + self._append_various_line(line)
> + else:
> + assert False
In my review of v2, I suggested to use a function-valued variable for
the state machine. I explored this, and will send my findings
separately.
> +
> + def end_comment(self):
> + self._end_section()
> +
> + def _check_named_section(self, name):
> + if name in ('Returns:', 'Since:',
> + # those are often singular or plural
> + 'Note:', 'Notes:',
> + 'Example:', 'Examples:',
> + 'TODO:'):
> + self._part = QAPIDoc.DocPart.VARIOUS
> + return True
> + return False
The side effect hidden in this function confused me once again, so I
played with the code to see whether avoiding it would be bothersome.
Turns out it isn't, proposed fixup appended.
> +
> + def _append_body_line(self, line):
> + name = line.split(' ', 1)[0]
> # FIXME not nice: things like '# @foo:' and '# @foo: ' aren't
> # recognized, and get silently treated as ordinary text
> - if self.symbol:
> - self._append_symbol_line(line)
> - elif not self.body.text and line.startswith('@'):
> + if not self.symbol and not self.body.text and line.startswith('@'):
> if not line.endswith(':'):
> raise QAPIParseError(self._parser, "Line should end with :")
> self.symbol = line[1:-1]
> # FIXME invalid names other than the empty string aren't flagged
> if not self.symbol:
> raise QAPIParseError(self._parser, "Invalid name")
> + elif self.symbol:
> + # We already know that we document some symbol
> + if name.startswith('@') and name.endswith(':'):
> + self._part = QAPIDoc.DocPart.ARGS
> + self._append_args_line(line)
> + elif self.symbol and self._check_named_section(name):
Checking self.symbol again is redundant.
> + self._append_various_line(line)
> + else:
> + self._append_freeform(line.strip())
> else:
> - self._append_freeform(line)
> -
> - def end_comment(self):
> - self._end_section()
> + # This is free-form documentation without a symbol
> + self._append_freeform(line.strip())
>
> - def _append_symbol_line(self, line):
> + def _append_args_line(self, line):
> name = line.split(' ', 1)[0]
>
> if name.startswith('@') and name.endswith(':'):
> line = line[len(name)+1:]
> self._start_args_section(name[1:-1])
> - elif name in ('Returns:', 'Since:',
> - # those are often singular or plural
> - 'Note:', 'Notes:',
> - 'Example:', 'Examples:',
> - 'TODO:'):
> + elif self._check_named_section(name):
> + self._append_various_line(line)
> + return
> + elif (self._section.text.endswith('\n\n')
> + and line and not line[0].isspace()):
> + self._start_section()
> + self._part = QAPIDoc.DocPart.VARIOUS
> + self._append_various_line(line)
> + return
> +
> + self._append_freeform(line.strip())
> +
> + def _append_various_line(self, line):
> + name = line.split(' ', 1)[0]
> +
> + if name.startswith('@') and name.endswith(':'):
> + raise QAPIParseError(self._parser,
> + "'%s' can't follow '%s' section"
> + % (name, self.sections[0].name))
> + elif self._check_named_section(name):
> line = line[len(name)+1:]
> self._start_section(name[:-1])
>
> + if (not self._section.name or
> + not self._section.name.startswith('Example')):
> + line = line.strip()
> +
> self._append_freeform(line)
>
> def _start_args_section(self, name):
> @@ -194,10 +276,7 @@ class QAPIDoc(object):
> if name in self.args:
> raise QAPIParseError(self._parser,
> "'%s' parameter name duplicated" % name)
> - if self.sections:
> - raise QAPIParseError(self._parser,
> - "'@%s:' can't follow '%s' section"
> - % (name, self.sections[0].name))
> + assert not self.sections
> self._end_section()
> self._section = QAPIDoc.ArgSection(name)
> self.args[name] = self._section
> @@ -219,13 +298,6 @@ class QAPIDoc(object):
> self._section = None
>
> def _append_freeform(self, line):
> - in_arg = isinstance(self._section, QAPIDoc.ArgSection)
> - if (in_arg and self._section.text.endswith('\n\n')
> - and line and not line[0].isspace()):
> - self._start_section()
> - if (in_arg or not self._section.name
> - or not self._section.name.startswith('Example')):
> - line = line.strip()
> match = re.match(r'(@\S+:)', line)
> if match:
> raise QAPIParseError(self._parser,
diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index 55ccd216cf..b42dad9b36 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -200,15 +200,13 @@ class QAPIDoc(object):
def end_comment(self):
self._end_section()
- def _check_named_section(self, name):
- if name in ('Returns:', 'Since:',
- # those are often singular or plural
- 'Note:', 'Notes:',
- 'Example:', 'Examples:',
- 'TODO:'):
- self._part = QAPIDoc.DocPart.VARIOUS
- return True
- return False
+ @staticmethod
+ def _is_section_tag(name):
+ return name in ('Returns:', 'Since:',
+ # those are often singular or plural
+ 'Note:', 'Notes:',
+ 'Example:', 'Examples:',
+ 'TODO:')
def _append_body_line(self, line):
name = line.split(' ', 1)[0]
@@ -226,7 +224,8 @@ class QAPIDoc(object):
if name.startswith('@') and name.endswith(':'):
self._part = QAPIDoc.DocPart.ARGS
self._append_args_line(line)
- elif self.symbol and self._check_named_section(name):
+ elif self._is_section_tag(name):
+ self._part = QAPIDoc.DocPart.VARIOUS
self._append_various_line(line)
else:
self._append_freeform(line.strip())
@@ -240,7 +239,8 @@ class QAPIDoc(object):
if name.startswith('@') and name.endswith(':'):
line = line[len(name)+1:]
self._start_args_section(name[1:-1])
- elif self._check_named_section(name):
+ elif self._is_section_tag(name):
+ self._part = QAPIDoc.DocPart.VARIOUS
self._append_various_line(line)
return
elif (self._section.text.endswith('\n\n')
@@ -259,7 +259,7 @@ class QAPIDoc(object):
raise QAPIParseError(self._parser,
"'%s' can't follow '%s' section"
% (name, self.sections[0].name))
- elif self._check_named_section(name):
+ elif self._is_section_tag(name):
line = line[len(name)+1:]
self._start_section(name[:-1])
