[Qemu-devel] [PATCH v4 4/6] qapi: Disentangle QAPIDoc code

Kevin Wolf posted 6 patches 6 years, 8 months ago
Maintainers: Michael Roth <mdroth@linux.vnet.ibm.com>, Eric Blake <eblake@redhat.com>, Markus Armbruster <armbru@redhat.com>
[Qemu-devel] [PATCH v4 4/6] qapi: Disentangle QAPIDoc code
Posted by Kevin Wolf 6 years, 8 months ago
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 <kwolf@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:
+        """
+        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.
+        """
         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
+
+    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
+
+    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):
+                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,
-- 
2.20.1


Re: [Qemu-devel] [PATCH v4 4/6] qapi: Disentangle QAPIDoc code
Posted by Markus Armbruster 6 years, 8 months ago
Kevin Wolf <kwolf@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 <kwolf@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])
 

[Qemu-devel] [PATCH v4 4.5/6] qapi: Replace QAPIDoc._part by ._append_line, and rework comments
Posted by Markus Armbruster 6 years, 8 months ago
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
This is on top of the fixup I appended to my review of v4.  I'd squash
all three patches together.

The next patch needs to be updated for this.

Unsquashed branch at git://repo.or.cz/qemu/armbru.git branch
qapi-features.

Let me know what you think.

 scripts/qapi/common.py | 109 ++++++++++++++++++++++++-----------------
 1 file changed, 63 insertions(+), 46 deletions(-)

diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index b42dad9b36..004b68df5c 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -102,6 +102,22 @@ class QAPISemError(QAPIError):
 
 
 class QAPIDoc(object):
+    """
+    A documentation comment block, either expression or free-form
+
+    Expression documentation blocks consist of
+
+    * a body section: one line naming the expression, followed by an
+      overview (any number of lines)
+
+    * argument sections: a description of each argument (for commands
+      and events) or member (for structs, unions and alternates)
+
+    * additional (non-argument) sections, possibly tagged
+
+    Free-form documentation blocks consist only of a body section.
+    """
+
     class Section(object):
         def __init__(self, name=None):
             # optional section name (argument/member or section name)
@@ -120,22 +136,6 @@ class QAPIDoc(object):
         def connect(self, member):
             self.member = member
 
-    class DocPart:
-        """
-        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.
@@ -151,7 +151,7 @@ class QAPIDoc(object):
         self.sections = []
         # the current section
         self._section = self.body
-        self._part = QAPIDoc.DocPart.BODY
+        self._append_line = self._append_body_line
 
     def has_section(self, name):
         """Return True if we have a section with this name."""
@@ -164,20 +164,11 @@ class QAPIDoc(object):
         """
         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.
+        The way that the line is dealt with depends on which part of
+        the documentation we're parsing right now:
+        * The body section: ._append_line is ._append_body_line
+        * An argument section: ._append_line is ._append_args_line
+        * An additional section: ._append_line is ._append_various_line
         """
         line = line[1:]
         if not line:
@@ -187,15 +178,7 @@ class QAPIDoc(object):
         if line[0] != ' ':
             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
+        self._append_line(line)
 
     def end_comment(self):
         self._end_section()
@@ -209,6 +192,19 @@ class QAPIDoc(object):
                         'TODO:')
 
     def _append_body_line(self, line):
+        """
+        Process a line of documentation text in the body section.
+
+        If this a symbol line and it is the section's first line, this
+        is an expression documentation block for that symbol.
+
+        If it's an expression documentation block, another symbol line
+        begins the argument section for the argument named by it, and
+        a section tag begins an additional section.  Start that
+        section and append the line to it.
+
+        Else, append the line to the current section.
+        """
         name = line.split(' ', 1)[0]
         # FIXME not nice: things like '#  @foo:' and '# @foo: ' aren't
         # recognized, and get silently treated as ordinary text
@@ -220,39 +216,60 @@ class QAPIDoc(object):
             if not self.symbol:
                 raise QAPIParseError(self._parser, "Invalid name")
         elif self.symbol:
-            # We already know that we document some symbol
+            # This is an expression documentation block
             if name.startswith('@') and name.endswith(':'):
-                self._part = QAPIDoc.DocPart.ARGS
+                self._append_line = self._append_args_line
                 self._append_args_line(line)
             elif self._is_section_tag(name):
-                self._part = QAPIDoc.DocPart.VARIOUS
+                self._append_line = self._append_various_line
                 self._append_various_line(line)
             else:
                 self._append_freeform(line.strip())
         else:
-            # This is free-form documentation without a symbol
+            # This is a free-form documentation block
             self._append_freeform(line.strip())
 
     def _append_args_line(self, line):
+        """
+        Process a line of documentation text in an argument section.
+
+        A symbol line begins the next argument section, a section tag
+        section or a non-indented line after a blank line begins an
+        additional section.  Start that section and append the line to
+        it.
+
+        Else, append the line to the current section.
+
+        """
         name = line.split(' ', 1)[0]
 
         if name.startswith('@') and name.endswith(':'):
             line = line[len(name)+1:]
             self._start_args_section(name[1:-1])
         elif self._is_section_tag(name):
-            self._part = QAPIDoc.DocPart.VARIOUS
+            self._append_line = self._append_various_line
             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_line = self._append_various_line
             self._append_various_line(line)
             return
 
         self._append_freeform(line.strip())
 
     def _append_various_line(self, line):
+        """
+        Process a line of documentation text in an additional section.
+
+        A symbol line is an error.
+
+        A section tag begins an additional section.  Start that
+        section and append the line to it.
+
+        Else, append the line to the current section.
+        """
         name = line.split(' ', 1)[0]
 
         if name.startswith('@') and name.endswith(':'):
-- 
2.21.0


Re: [Qemu-devel] [PATCH v4 4.5/6] qapi: Replace QAPIDoc._part by ._append_line, and rework comments
Posted by Kevin Wolf 6 years, 8 months ago
Am 06.06.2019 um 14:01 hat Markus Armbruster geschrieben:
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> This is on top of the fixup I appended to my review of v4.  I'd squash
> all three patches together.
> 
> The next patch needs to be updated for this.
> 
> Unsquashed branch at git://repo.or.cz/qemu/armbru.git branch
> qapi-features.
> 
> Let me know what you think.

As you know, I don't like the self._append_line function pointer and
think it makes the code less readable. However, I also hope that I'll
never have to touch this code again, whereas you as the maintainer will
probably have to. So your taste is more imporant than mine.

So for all I care, go ahead and squash in your changes.

Kevin