qapi: Relax doc string @name: description indentation rules

The QAPI schema doc comment language provides special syntax for
command and event arguments, struct and union members, alternate
branches, enumeration values, and features: descriptions starting with
"@name:".

By convention, we format them like this:

    # @name: Lorem ipsum dolor sit amet, consectetur adipiscing elit,
    #        sed do eiusmod tempor incididunt ut labore et dolore
    #        magna aliqua.

Okay for names as short as "name", but we have much longer ones.  Their
description gets squeezed against the right margin, like this:

    # @dirty-sync-missed-zero-copy: Number of times dirty RAM synchronization could
    #                               not avoid copying dirty pages. This is between
    #                               0 and @dirty-sync-count * @multifd-channels.
    #                               (since 7.1)

The description text is effectively just 50 characters wide.  Easy
enough to read, but can be cumbersome to write.

The awkward squeeze against the right margin makes people go beyond it,
which produces two undesirables: arguments about style, and descriptions
that are unnecessarily hard to read, like this one:

    # @postcopy-vcpu-blocktime: list of the postcopy blocktime per vCPU.  This is
    #                           only present when the postcopy-blocktime migration capability
    #                           is enabled. (Since 3.0)

We could instead format it like

    # @postcopy-vcpu-blocktime:
    # list of the postcopy blocktime per vCPU.  This is only present
    # when the postcopy-blocktime migration capability is
    # enabled. (Since 3.0)

or, since the commit before previous, like

    # @postcopy-vcpu-blocktime:
    # 	  list of the postcopy blocktime per vCPU.  This is only present
    # 	  when the postcopy-blocktime migration capability is
    # 	  enabled. (Since 3.0)

However, I'd rather have

    # @postcopy-vcpu-blocktime: list of the postcopy blocktime per vCPU.
    #     This is only present when the postcopy-blocktime migration
    #     capability is enabled.  (Since 3.0)

because this is how rST field and option lists work.

To get this, we need to let the first non-blank line after the
"@name:" line determine expected indentation.

This fills up the indentation pitfall mentioned in
docs/devel/qapi-code-gen.rst.  A related pitfall still exists.  Update
the text to show it.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20230428105429.1687850-14-armbru@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
[Work around lack of walrus operator in Python 3.7 and older]

diff --git a/docs/devel/qapi-code-gen.rst b/docs/devel/qapi-code-gen.rst
index 6386b588819bbc944ed780836477387c6d09b51a..875f893be22a2f58a73ab268db15decfadf0e626 100644 (file)
--- a/docs/devel/qapi-code-gen.rst
+++ b/docs/devel/qapi-code-gen.rst
@@ -1074,10 +1074,14 @@ Indentation matters.  Bad example::
 
  # @none: None (no memory side cache in this proximity domain,
  #              or cache associativity unknown)
+ #     (since 5.0)
 
-The description is parsed as a definition list with term "None (no
-memory side cache in this proximity domain," and definition "or cache
-associativity unknown)".
+The last line's de-indent is wrong.  The second and subsequent lines
+need to line up with each other, like this::
+
+ # @none: None (no memory side cache in this proximity domain,
+ #     or cache associativity unknown)
+ #     (since 5.0)
 
 Section tags are case-sensitive and end with a colon.  Good example::
 

diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py
index a4ff9b6dbfa64bb726287b52809951914226fe32..285c9ff4c95b44d995a81d79a3b84e28c20cbf80 100644 (file)
--- a/scripts/qapi/parser.py
+++ b/scripts/qapi/parser.py
@@ -474,17 +474,21 @@ class QAPIDoc:
             self._parser = parser
             # optional section name (argument/member or section name)
             self.name = name
+            # section text without section name
             self.text = ''
-            # the expected indent level of the text of this section
-            self._indent = indent
+            # indentation to strip (None means indeterminate)
+            self._indent = None if self.name else 0
 
         def append(self, line: str) -> None:
-            # Strip leading spaces corresponding to the expected indent level
-            # Blank lines are always OK.
+            line = line.rstrip()
+
             if line:
                 indent = must_match(r'\s*', line).end()
-                if self._indent < 0:
-                    self._indent = indent
+                if self._indent is None:
+                    # indeterminate indentation
+                    if self.text != '':
+                        # non-blank, non-first line determines indentation
+                        self._indent = indent
                 elif indent < self._indent:
                     raise QAPIParseError(
                         self._parser,
@@ -492,7 +496,7 @@ class QAPIDoc:
                         self._indent)
                 line = line[self._indent:]
 
-            self.text += line.rstrip() + '\n'
+            self.text += line + '\n'
 
     class ArgSection(Section):
         def __init__(self, parser: QAPISchemaParser,
@@ -622,22 +626,8 @@ class QAPIDoc:
         """
         match = self._match_at_name_colon(line)
         if match:
-            # 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.
-            name = match.group(1)
-            indent = match.end()
-            line = line[indent:]
-            if not line:
-                # Line was just the "@arg:" header
-                # The next non-blank line determines expected indent
-                indent = -1
-            else:
-                line = ' ' * indent + line
-            self._start_args_section(name, indent)
+            line = line[match.end():]
+            self._start_args_section(match.group(1), 0)
         elif self._match_section_tag(line):
             self._append_line = self._append_various_line
             self._append_various_line(line)
@@ -657,22 +647,8 @@ class QAPIDoc:
     def _append_features_line(self, line: str) -> None:
         match = self._match_at_name_colon(line)
         if match:
-            # 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.
-            name = match.group(1)
-            indent = match.end()
-            line = line[indent:]
-            if not line:
-                # Line was just the "@arg:" header
-                # The next non-blank line determines expected indent
-                indent = -1
-            else:
-                line = ' ' * indent + line
-            self._start_features_section(name, indent)
+            line = line[match.end():]
+            self._start_features_section(match.group(1), 0)
         elif self._match_section_tag(line):
             self._append_line = self._append_various_line
             self._append_various_line(line)
@@ -704,21 +680,8 @@ class QAPIDoc:
                                  % (match.group(1), self.sections[0].name))
         match = self._match_section_tag(line)
         if match:
-            # 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 = must_match(r'\S*:\s*', line).end()
-            line = line[indent:]
-            if not line:
-                # Line was just the "Section:" header
-                # The next non-blank line determines expected indent
-                indent = 0
-            else:
-                line = ' ' * indent + line
-            self._start_section(match.group(1), indent)
+            line = line[match.end():]
+            self._start_section(match.group(1), 0)
 
         self._append_freeform(line)
 
@@ -754,7 +717,7 @@ class QAPIDoc:
         self.sections.append(new_section)
 
     def _switch_section(self, new_section: 'QAPIDoc.Section') -> None:
-        text = self._section.text = self._section.text.strip()
+        text = self._section.text = self._section.text.strip('\n')
 
         # Only the 'body' section is allowed to have an empty body.
         # All other sections, including anonymous ones, must have text.

diff --git a/tests/qapi-schema/doc-bad-indent.err b/tests/qapi-schema/doc-bad-indent.err
index 67844539bd215e0c57df76e78380dd6a1dc9b18b..3c9699a8e0fd4cf16b95fa1258d5b7beed734c1f 100644 (file)
--- a/tests/qapi-schema/doc-bad-indent.err
+++ b/tests/qapi-schema/doc-bad-indent.err
@@ -1 +1 @@
-doc-bad-indent.json:6:1: unexpected de-indent (expected at least 4 spaces)
+doc-bad-indent.json:7:1: unexpected de-indent (expected at least 2 spaces)

diff --git a/tests/qapi-schema/doc-bad-indent.json b/tests/qapi-schema/doc-bad-indent.json
index edde8f21dc7104222c85ecdb11615fb6ccba910c..3f22a2771733b3f5825cb77f9a02116513a85b22 100644 (file)
--- a/tests/qapi-schema/doc-bad-indent.json
+++ b/tests/qapi-schema/doc-bad-indent.json
@@ -3,6 +3,7 @@
 ##
 # @foo:
 # @a: line one
-# line two is wrongly indented
+#   line two
+# line three is wrongly indented
 ##
 { 'command': 'foo', 'data': { 'a': 'int' } }

diff --git a/tests/qapi-schema/doc-good.json b/tests/qapi-schema/doc-good.json
index 34c3dcbe97cf8c0eb9b3266a6a9f29983fd61820..354dfdf461638753434efd23d8ee43d0e09f3d83 100644 (file)
--- a/tests/qapi-schema/doc-good.json
+++ b/tests/qapi-schema/doc-good.json
@@ -144,7 +144,8 @@
 #     description starts on a new line,
 #     indented
 #
-# @arg2: the second argument
+# @arg2: description starts on the same line
+#     remainder indented differently
 #
 # Features:
 # @cmd-feat1: a feature

diff --git a/tests/qapi-schema/doc-good.out b/tests/qapi-schema/doc-good.out
index 277371acc8d97f8b796c50367a1cf21a6df896cb..24d9ea954db7d3e5f517bf45a3f0d04baab40c24 100644 (file)
--- a/tests/qapi-schema/doc-good.out
+++ b/tests/qapi-schema/doc-good.out
@@ -161,7 +161,8 @@ doc symbol=cmd
 description starts on a new line,
 indented
     arg=arg2
-the second argument
+description starts on the same line
+remainder indented differently
     arg=arg3
 
     feature=cmd-feat1