[PATCH v5] qapi: Add documentation format validation

[PATCH v5] qapi: Add documentation format validation

[Vladimir Sementsov-Ogievskiy]

Add explicit validation for QAPI documentation formatting rules:

1. Lines must not exceed 70 columns in width (including '# ' prefix)
2. Sentences must be separated by two spaces

Example sections and literal :: blocks (seldom case) are excluded, we
don't require them to be <= 70, that would be too restrictive. Anyway,
they share common 80-columns recommendations (not requirements).

Add two simple tests, illustrating the change.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---

Hi all!

v5: - break "literal" block at any decreasing the indent,
      not only at no-indent
    - add two simple tests

This is based on
[PATCH 0/8] A QAPI schema doc markup fix, and style cleanup
Based-on: <20251031094751.2817932-1-armbru@redhat.com>

 scripts/qapi/parser.py                     | 52 +++++++++++++++++++++-
 tests/qapi-schema/doc-bad-long-line.err    |  1 +
 tests/qapi-schema/doc-bad-long-line.json   |  6 +++
 tests/qapi-schema/doc-bad-long-line.out    |  0
 tests/qapi-schema/doc-bad-whitespaces.err  |  2 +
 tests/qapi-schema/doc-bad-whitespaces.json |  6 +++
 tests/qapi-schema/doc-bad-whitespaces.out  |  0
 tests/qapi-schema/meson.build              |  2 +
 8 files changed, 68 insertions(+), 1 deletion(-)
 create mode 100644 tests/qapi-schema/doc-bad-long-line.err
 create mode 100644 tests/qapi-schema/doc-bad-long-line.json
 create mode 100644 tests/qapi-schema/doc-bad-long-line.out
 create mode 100644 tests/qapi-schema/doc-bad-whitespaces.err
 create mode 100644 tests/qapi-schema/doc-bad-whitespaces.json
 create mode 100644 tests/qapi-schema/doc-bad-whitespaces.out

diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py
index 9fbf80a541..ffb149850d 100644
--- a/scripts/qapi/parser.py
+++ b/scripts/qapi/parser.py
@@ -108,6 +108,11 @@ def __init__(self,
         self.exprs: List[QAPIExpression] = []
         self.docs: List[QAPIDoc] = []
 
+        # State for tracking qmp-example blocks and simple
+        # :: literal blocks.
+        self._literal_mode = False
+        self._literal_mode_indent = 0
+
         # Showtime!
         self._parse()
 
@@ -423,12 +428,57 @@ def get_doc_line(self) -> Optional[str]:
             if self.val != '##':
                 raise QAPIParseError(
                     self, "junk after '##' at end of documentation comment")
+            self._literal_mode = False
             return None
         if self.val == '#':
             return ''
         if self.val[1] != ' ':
             raise QAPIParseError(self, "missing space after #")
-        return self.val[2:].rstrip()
+
+        line = self.val[2:].rstrip()
+
+        if re.match(r'(\.\. +qmp-example)? *::$', line):
+            self._literal_mode = True
+            self._literal_mode_indent = 0
+        elif self._literal_mode and line:
+            indent = re.match(r'^ *', line).end()
+            if self._literal_mode_indent == 0:
+                self._literal_mode_indent = indent
+            elif indent < self._literal_mode_indent:
+                # ReST directives stop at decreasing indentation
+                self._literal_mode = False
+
+        if not self._literal_mode:
+            self._validate_doc_line_format(line)
+
+        return line
+
+    def _validate_doc_line_format(self, line: str) -> None:
+        """
+        Validate documentation format rules for a single line:
+        1. Lines should not exceed 70 columns
+        2. Sentences should be separated by two spaces
+        """
+        full_line_length = len(line) + 2  # "# " = 2 characters
+        if full_line_length > 70:
+            # Skip URL lines - they can't be broken
+            if re.match(r' *(https?|ftp)://[^ ]*$', line):
+                pass
+            else:
+                raise QAPIParseError(
+                    self, "documentation line exceeds 70 columns"
+                )
+
+        single_space_pattern = r'(\be\.g\.|^ *\d\.|([.!?])) [A-Z0-9(]'
+        for m in list(re.finditer(single_space_pattern, line)):
+            if not m.group(2):
+                continue
+            # HACK so the error message points to the offending spot
+            self.pos = self.line_pos + 2 + m.start(2) + 1
+            raise QAPIParseError(
+                self, "Use two spaces between sentences\n"
+                "If this not the end of a sentence, please report the bug",
+            )
 
     @staticmethod
     def _match_at_name_colon(string: str) -> Optional[Match[str]]:
diff --git a/tests/qapi-schema/doc-bad-long-line.err b/tests/qapi-schema/doc-bad-long-line.err
new file mode 100644
index 0000000000..611a3b1fef
--- /dev/null
+++ b/tests/qapi-schema/doc-bad-long-line.err
@@ -0,0 +1 @@
+doc-bad-long-line.json:4:1: documentation line exceeds 70 columns
diff --git a/tests/qapi-schema/doc-bad-long-line.json b/tests/qapi-schema/doc-bad-long-line.json
new file mode 100644
index 0000000000..d7f887694d
--- /dev/null
+++ b/tests/qapi-schema/doc-bad-long-line.json
@@ -0,0 +1,6 @@
+##
+# @foo:
+#
+# This line has exactly 71 characters, including spaces and punctuation!
+##
+{ 'command': 'foo' }
diff --git a/tests/qapi-schema/doc-bad-long-line.out b/tests/qapi-schema/doc-bad-long-line.out
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/tests/qapi-schema/doc-bad-whitespaces.err b/tests/qapi-schema/doc-bad-whitespaces.err
new file mode 100644
index 0000000000..5cca1954c0
--- /dev/null
+++ b/tests/qapi-schema/doc-bad-whitespaces.err
@@ -0,0 +1,2 @@
+doc-bad-whitespaces.json:4:48: Use two spaces between sentences
+If this not the end of a sentence, please report the bug
diff --git a/tests/qapi-schema/doc-bad-whitespaces.json b/tests/qapi-schema/doc-bad-whitespaces.json
new file mode 100644
index 0000000000..b0c318c670
--- /dev/null
+++ b/tests/qapi-schema/doc-bad-whitespaces.json
@@ -0,0 +1,6 @@
+##
+# @foo:
+#
+# Sentences should be split by two whitespaces. But here is only one.
+##
+{ 'command': 'foo' }
diff --git a/tests/qapi-schema/doc-bad-whitespaces.out b/tests/qapi-schema/doc-bad-whitespaces.out
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/tests/qapi-schema/meson.build b/tests/qapi-schema/meson.build
index c47025d16d..b24b27db21 100644
--- a/tests/qapi-schema/meson.build
+++ b/tests/qapi-schema/meson.build
@@ -61,8 +61,10 @@ schemas = [
   'doc-bad-event-arg.json',
   'doc-bad-feature.json',
   'doc-bad-indent.json',
+  'doc-bad-long-line.json',
   'doc-bad-symbol.json',
   'doc-bad-union-member.json',
+  'doc-bad-whitespaces.json',
   'doc-before-include.json',
   'doc-before-pragma.json',
   'doc-duplicate-features.json',
-- 
2.48.1

Re: [PATCH v5] qapi: Add documentation format validation

[Markus Armbruster]

Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:

> Add explicit validation for QAPI documentation formatting rules:
>
> 1. Lines must not exceed 70 columns in width (including '# ' prefix)
> 2. Sentences must be separated by two spaces
>
> Example sections and literal :: blocks (seldom case) are excluded, we
> don't require them to be <= 70, that would be too restrictive. Anyway,
> they share common 80-columns recommendations (not requirements).
>
> Add two simple tests, illustrating the change.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> ---
>
> Hi all!
>
> v5: - break "literal" block at any decreasing the indent,
>       not only at no-indent
>     - add two simple tests
>
> This is based on
> [PATCH 0/8] A QAPI schema doc markup fix, and style cleanup
> Based-on: <20251031094751.2817932-1-armbru@redhat.com>
>
>  scripts/qapi/parser.py                     | 52 +++++++++++++++++++++-
>  tests/qapi-schema/doc-bad-long-line.err    |  1 +
>  tests/qapi-schema/doc-bad-long-line.json   |  6 +++
>  tests/qapi-schema/doc-bad-long-line.out    |  0
>  tests/qapi-schema/doc-bad-whitespaces.err  |  2 +
>  tests/qapi-schema/doc-bad-whitespaces.json |  6 +++
>  tests/qapi-schema/doc-bad-whitespaces.out  |  0
>  tests/qapi-schema/meson.build              |  2 +
>  8 files changed, 68 insertions(+), 1 deletion(-)
>  create mode 100644 tests/qapi-schema/doc-bad-long-line.err
>  create mode 100644 tests/qapi-schema/doc-bad-long-line.json
>  create mode 100644 tests/qapi-schema/doc-bad-long-line.out
>  create mode 100644 tests/qapi-schema/doc-bad-whitespaces.err
>  create mode 100644 tests/qapi-schema/doc-bad-whitespaces.json
>  create mode 100644 tests/qapi-schema/doc-bad-whitespaces.out
>
> diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py
> index 9fbf80a541..ffb149850d 100644
> --- a/scripts/qapi/parser.py
> +++ b/scripts/qapi/parser.py
> @@ -108,6 +108,11 @@ def __init__(self,
>          self.exprs: List[QAPIExpression] = []
>          self.docs: List[QAPIDoc] = []
>  
> +        # State for tracking qmp-example blocks and simple
> +        # :: literal blocks.
> +        self._literal_mode = False
> +        self._literal_mode_indent = 0
> +
>          # Showtime!
>          self._parse()
>  
> @@ -423,12 +428,57 @@ def get_doc_line(self) -> Optional[str]:
>              if self.val != '##':
>                  raise QAPIParseError(
>                      self, "junk after '##' at end of documentation comment")
> +            self._literal_mode = False
>              return None
>          if self.val == '#':
>              return ''
>          if self.val[1] != ' ':
>              raise QAPIParseError(self, "missing space after #")
> -        return self.val[2:].rstrip()
> +
> +        line = self.val[2:].rstrip()
> +
> +        if re.match(r'(\.\. +qmp-example)? *::$', line):

After a closer reading of ReST docs and some testing: this isn't quite
right, although it works okay for what we have.

A directive's '::' need not be at the end of a line.  The regexp fails
to match a qmp-example directive with text after '::'.  No such
directives exist right now.

A literal block starts after a '::' at the end of a paragraph,
i.e. after '::' and a blank line.  The regexp only matches '::' on its
own line, not at the end of a line of text.  It matches it even when
it's not followed by a blank line.

In review of v4, I claimed we don't use the contracted form "text::".
Not true.  For instance, in block-core.json:

   # @bins: list of io request counts corresponding to histogram
   #     intervals, one more element than @boundaries has.  For the
   #     example above, @bins may be something like [3, 1, 5, 2], and
   #     corresponding histogram looks like::
   #
   #        5|           *
   #        4|           *
   #        3| *         *
   #        2| *         *    *
   #        1| *    *    *    *
   #         +------------------
   #             10   50   100
   #
   # Since: 4.0
   ##

The literal block starts after "like::" and ends before "Since:'.

> +            self._literal_mode = True
> +            self._literal_mode_indent = 0
> +        elif self._literal_mode and line:
> +            indent = re.match(r'^ *', line).end()
> +            if self._literal_mode_indent == 0:
> +                self._literal_mode_indent = indent
> +            elif indent < self._literal_mode_indent:
> +                # ReST directives stop at decreasing indentation
> +                self._literal_mode = False

This isn't quite right, either.  We need to stop when indentation of
non-blank lines drops below the indentation of the line containing the
'::'.

Perhaps it's easier for both of us if I fix this on top.  Thoughts?

> +
> +        if not self._literal_mode:
> +            self._validate_doc_line_format(line)
> +
> +        return line
> +
> +    def _validate_doc_line_format(self, line: str) -> None:
> +        """
> +        Validate documentation format rules for a single line:
> +        1. Lines should not exceed 70 columns
> +        2. Sentences should be separated by two spaces
> +        """
> +        full_line_length = len(line) + 2  # "# " = 2 characters
> +        if full_line_length > 70:
> +            # Skip URL lines - they can't be broken
> +            if re.match(r' *(https?|ftp)://[^ ]*$', line):
> +                pass
> +            else:
> +                raise QAPIParseError(
> +                    self, "documentation line exceeds 70 columns"
> +                )
> +
> +        single_space_pattern = r'(\be\.g\.|^ *\d\.|([.!?])) [A-Z0-9(]'
> +        for m in list(re.finditer(single_space_pattern, line)):
> +            if not m.group(2):
> +                continue
> +            # HACK so the error message points to the offending spot
> +            self.pos = self.line_pos + 2 + m.start(2) + 1
> +            raise QAPIParseError(
> +                self, "Use two spaces between sentences\n"
> +                "If this not the end of a sentence, please report the bug",
> +            )
>  
>      @staticmethod
>      def _match_at_name_colon(string: str) -> Optional[Match[str]]:
> diff --git a/tests/qapi-schema/doc-bad-long-line.err b/tests/qapi-schema/doc-bad-long-line.err
> new file mode 100644
> index 0000000000..611a3b1fef
> --- /dev/null
> +++ b/tests/qapi-schema/doc-bad-long-line.err
> @@ -0,0 +1 @@
> +doc-bad-long-line.json:4:1: documentation line exceeds 70 columns
> diff --git a/tests/qapi-schema/doc-bad-long-line.json b/tests/qapi-schema/doc-bad-long-line.json
> new file mode 100644
> index 0000000000..d7f887694d
> --- /dev/null
> +++ b/tests/qapi-schema/doc-bad-long-line.json
> @@ -0,0 +1,6 @@
> +##
> +# @foo:
> +#
> +# This line has exactly 71 characters, including spaces and punctuation!

Really?

> +##
> +{ 'command': 'foo' }
> diff --git a/tests/qapi-schema/doc-bad-long-line.out b/tests/qapi-schema/doc-bad-long-line.out
> new file mode 100644
> index 0000000000..e69de29bb2
> diff --git a/tests/qapi-schema/doc-bad-whitespaces.err b/tests/qapi-schema/doc-bad-whitespaces.err
> new file mode 100644
> index 0000000000..5cca1954c0
> --- /dev/null
> +++ b/tests/qapi-schema/doc-bad-whitespaces.err
> @@ -0,0 +1,2 @@
> +doc-bad-whitespaces.json:4:48: Use two spaces between sentences
> +If this not the end of a sentence, please report the bug
> diff --git a/tests/qapi-schema/doc-bad-whitespaces.json b/tests/qapi-schema/doc-bad-whitespaces.json
> new file mode 100644
> index 0000000000..b0c318c670
> --- /dev/null
> +++ b/tests/qapi-schema/doc-bad-whitespaces.json
> @@ -0,0 +1,6 @@
> +##
> +# @foo:
> +#
> +# Sentences should be split by two whitespaces. But here is only one.

two spaces

> +##
> +{ 'command': 'foo' }
> diff --git a/tests/qapi-schema/doc-bad-whitespaces.out b/tests/qapi-schema/doc-bad-whitespaces.out
> new file mode 100644
> index 0000000000..e69de29bb2
> diff --git a/tests/qapi-schema/meson.build b/tests/qapi-schema/meson.build
> index c47025d16d..b24b27db21 100644
> --- a/tests/qapi-schema/meson.build
> +++ b/tests/qapi-schema/meson.build
> @@ -61,8 +61,10 @@ schemas = [
>    'doc-bad-event-arg.json',
>    'doc-bad-feature.json',
>    'doc-bad-indent.json',
> +  'doc-bad-long-line.json',
>    'doc-bad-symbol.json',
>    'doc-bad-union-member.json',
> +  'doc-bad-whitespaces.json',
>    'doc-before-include.json',
>    'doc-before-pragma.json',
>    'doc-duplicate-features.json',

Re: [PATCH v5] qapi: Add documentation format validation

[Vladimir Sementsov-Ogievskiy]

On 04.11.25 13:07, Markus Armbruster wrote:
> Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:
> 
>> Add explicit validation for QAPI documentation formatting rules:
>>
>> 1. Lines must not exceed 70 columns in width (including '# ' prefix)
>> 2. Sentences must be separated by two spaces
>>
>> Example sections and literal :: blocks (seldom case) are excluded, we
>> don't require them to be <= 70, that would be too restrictive. Anyway,
>> they share common 80-columns recommendations (not requirements).
>>
>> Add two simple tests, illustrating the change.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
>> ---
>>
>> Hi all!
>>
>> v5: - break "literal" block at any decreasing the indent,
>>        not only at no-indent
>>      - add two simple tests
>>
>> This is based on
>> [PATCH 0/8] A QAPI schema doc markup fix, and style cleanup
>> Based-on: <20251031094751.2817932-1-armbru@redhat.com>
>>
>>   scripts/qapi/parser.py                     | 52 +++++++++++++++++++++-
>>   tests/qapi-schema/doc-bad-long-line.err    |  1 +
>>   tests/qapi-schema/doc-bad-long-line.json   |  6 +++
>>   tests/qapi-schema/doc-bad-long-line.out    |  0
>>   tests/qapi-schema/doc-bad-whitespaces.err  |  2 +
>>   tests/qapi-schema/doc-bad-whitespaces.json |  6 +++
>>   tests/qapi-schema/doc-bad-whitespaces.out  |  0
>>   tests/qapi-schema/meson.build              |  2 +
>>   8 files changed, 68 insertions(+), 1 deletion(-)
>>   create mode 100644 tests/qapi-schema/doc-bad-long-line.err
>>   create mode 100644 tests/qapi-schema/doc-bad-long-line.json
>>   create mode 100644 tests/qapi-schema/doc-bad-long-line.out
>>   create mode 100644 tests/qapi-schema/doc-bad-whitespaces.err
>>   create mode 100644 tests/qapi-schema/doc-bad-whitespaces.json
>>   create mode 100644 tests/qapi-schema/doc-bad-whitespaces.out
>>
>> diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py
>> index 9fbf80a541..ffb149850d 100644
>> --- a/scripts/qapi/parser.py
>> +++ b/scripts/qapi/parser.py
>> @@ -108,6 +108,11 @@ def __init__(self,
>>           self.exprs: List[QAPIExpression] = []
>>           self.docs: List[QAPIDoc] = []
>>   
>> +        # State for tracking qmp-example blocks and simple
>> +        # :: literal blocks.
>> +        self._literal_mode = False
>> +        self._literal_mode_indent = 0
>> +
>>           # Showtime!
>>           self._parse()
>>   
>> @@ -423,12 +428,57 @@ def get_doc_line(self) -> Optional[str]:
>>               if self.val != '##':
>>                   raise QAPIParseError(
>>                       self, "junk after '##' at end of documentation comment")
>> +            self._literal_mode = False
>>               return None
>>           if self.val == '#':
>>               return ''
>>           if self.val[1] != ' ':
>>               raise QAPIParseError(self, "missing space after #")
>> -        return self.val[2:].rstrip()
>> +
>> +        line = self.val[2:].rstrip()
>> +
>> +        if re.match(r'(\.\. +qmp-example)? *::$', line):
> 
> After a closer reading of ReST docs and some testing: this isn't quite
> right, although it works okay for what we have.
> 
> A directive's '::' need not be at the end of a line.  The regexp fails
> to match a qmp-example directive with text after '::'.  No such
> directives exist right now.
> 
> A literal block starts after a '::' at the end of a paragraph,
> i.e. after '::' and a blank line.  The regexp only matches '::' on its
> own line, not at the end of a line of text.  It matches it even when
> it's not followed by a blank line.
> 
> In review of v4, I claimed we don't use the contracted form "text::".
> Not true.  For instance, in block-core.json:
> 
>     # @bins: list of io request counts corresponding to histogram
>     #     intervals, one more element than @boundaries has.  For the
>     #     example above, @bins may be something like [3, 1, 5, 2], and
>     #     corresponding histogram looks like::
>     #
>     #        5|           *
>     #        4|           *
>     #        3| *         *
>     #        2| *         *    *
>     #        1| *    *    *    *
>     #         +------------------
>     #             10   50   100
>     #

Ow, my old histograms)

>     # Since: 4.0
>     ##
> 
> The literal block starts after "like::" and ends before "Since:'.
> 
>> +            self._literal_mode = True
>> +            self._literal_mode_indent = 0
>> +        elif self._literal_mode and line:
>> +            indent = re.match(r'^ *', line).end()
>> +            if self._literal_mode_indent == 0:
>> +                self._literal_mode_indent = indent
>> +            elif indent < self._literal_mode_indent:
>> +                # ReST directives stop at decreasing indentation
>> +                self._literal_mode = False
> 
> This isn't quite right, either.  We need to stop when indentation of
> non-blank lines drops below the indentation of the line containing the
> '::'.
> 
> Perhaps it's easier for both of us if I fix this on top.  Thoughts?

No objections, good for me!

> 
>> +
>> +        if not self._literal_mode:
>> +            self._validate_doc_line_format(line)
>> +
>> +        return line
>> +
>> +    def _validate_doc_line_format(self, line: str) -> None:
>> +        """
>> +        Validate documentation format rules for a single line:
>> +        1. Lines should not exceed 70 columns
>> +        2. Sentences should be separated by two spaces
>> +        """
>> +        full_line_length = len(line) + 2  # "# " = 2 characters
>> +        if full_line_length > 70:
>> +            # Skip URL lines - they can't be broken
>> +            if re.match(r' *(https?|ftp)://[^ ]*$', line):
>> +                pass
>> +            else:
>> +                raise QAPIParseError(
>> +                    self, "documentation line exceeds 70 columns"
>> +                )
>> +
>> +        single_space_pattern = r'(\be\.g\.|^ *\d\.|([.!?])) [A-Z0-9(]'
>> +        for m in list(re.finditer(single_space_pattern, line)):
>> +            if not m.group(2):
>> +                continue
>> +            # HACK so the error message points to the offending spot
>> +            self.pos = self.line_pos + 2 + m.start(2) + 1
>> +            raise QAPIParseError(
>> +                self, "Use two spaces between sentences\n"
>> +                "If this not the end of a sentence, please report the bug",
>> +            )
>>   
>>       @staticmethod
>>       def _match_at_name_colon(string: str) -> Optional[Match[str]]:
>> diff --git a/tests/qapi-schema/doc-bad-long-line.err b/tests/qapi-schema/doc-bad-long-line.err
>> new file mode 100644
>> index 0000000000..611a3b1fef
>> --- /dev/null
>> +++ b/tests/qapi-schema/doc-bad-long-line.err
>> @@ -0,0 +1 @@
>> +doc-bad-long-line.json:4:1: documentation line exceeds 70 columns
>> diff --git a/tests/qapi-schema/doc-bad-long-line.json b/tests/qapi-schema/doc-bad-long-line.json
>> new file mode 100644
>> index 0000000000..d7f887694d
>> --- /dev/null
>> +++ b/tests/qapi-schema/doc-bad-long-line.json
>> @@ -0,0 +1,6 @@
>> +##
>> +# @foo:
>> +#
>> +# This line has exactly 71 characters, including spaces and punctuation!
> 
> Really?

Oh, it's 72 characters actually! AI tricked me. Didn't I check it out?

Maybe:

# This line has exactly 71 chars, including the leading hash and space.



> 
>> +##
>> +{ 'command': 'foo' }
>> diff --git a/tests/qapi-schema/doc-bad-long-line.out b/tests/qapi-schema/doc-bad-long-line.out
>> new file mode 100644
>> index 0000000000..e69de29bb2
>> diff --git a/tests/qapi-schema/doc-bad-whitespaces.err b/tests/qapi-schema/doc-bad-whitespaces.err
>> new file mode 100644
>> index 0000000000..5cca1954c0
>> --- /dev/null
>> +++ b/tests/qapi-schema/doc-bad-whitespaces.err
>> @@ -0,0 +1,2 @@
>> +doc-bad-whitespaces.json:4:48: Use two spaces between sentences
>> +If this not the end of a sentence, please report the bug
>> diff --git a/tests/qapi-schema/doc-bad-whitespaces.json b/tests/qapi-schema/doc-bad-whitespaces.json
>> new file mode 100644
>> index 0000000000..b0c318c670
>> --- /dev/null
>> +++ b/tests/qapi-schema/doc-bad-whitespaces.json
>> @@ -0,0 +1,6 @@
>> +##
>> +# @foo:
>> +#
>> +# Sentences should be split by two whitespaces. But here is only one.
> 
> two spaces
> 
>> +##
>> +{ 'command': 'foo' }
>> diff --git a/tests/qapi-schema/doc-bad-whitespaces.out b/tests/qapi-schema/doc-bad-whitespaces.out
>> new file mode 100644
>> index 0000000000..e69de29bb2
>> diff --git a/tests/qapi-schema/meson.build b/tests/qapi-schema/meson.build
>> index c47025d16d..b24b27db21 100644
>> --- a/tests/qapi-schema/meson.build
>> +++ b/tests/qapi-schema/meson.build
>> @@ -61,8 +61,10 @@ schemas = [
>>     'doc-bad-event-arg.json',
>>     'doc-bad-feature.json',
>>     'doc-bad-indent.json',
>> +  'doc-bad-long-line.json',
>>     'doc-bad-symbol.json',
>>     'doc-bad-union-member.json',
>> +  'doc-bad-whitespaces.json',
>>     'doc-before-include.json',
>>     'doc-before-pragma.json',
>>     'doc-duplicate-features.json',
> 


-- 
Best regards,
Vladimir

Re: [PATCH v5] qapi: Add documentation format validation

[Markus Armbruster]

Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:

> On 04.11.25 13:07, Markus Armbruster wrote:
>> Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:
>> 
>>> Add explicit validation for QAPI documentation formatting rules:
>>>
>>> 1. Lines must not exceed 70 columns in width (including '# ' prefix)
>>> 2. Sentences must be separated by two spaces
>>>
>>> Example sections and literal :: blocks (seldom case) are excluded, we
>>> don't require them to be <= 70, that would be too restrictive. Anyway,
>>> they share common 80-columns recommendations (not requirements).
>>>
>>> Add two simple tests, illustrating the change.
>>>
>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
>>> ---
>>>
>>> Hi all!
>>>
>>> v5: - break "literal" block at any decreasing the indent,
>>>        not only at no-indent
>>>      - add two simple tests
>>>
>>> This is based on
>>> [PATCH 0/8] A QAPI schema doc markup fix, and style cleanup
>>> Based-on: <20251031094751.2817932-1-armbru@redhat.com>
>>>
>>>   scripts/qapi/parser.py                     | 52 +++++++++++++++++++++-
>>>   tests/qapi-schema/doc-bad-long-line.err    |  1 +
>>>   tests/qapi-schema/doc-bad-long-line.json   |  6 +++
>>>   tests/qapi-schema/doc-bad-long-line.out    |  0
>>>   tests/qapi-schema/doc-bad-whitespaces.err  |  2 +
>>>   tests/qapi-schema/doc-bad-whitespaces.json |  6 +++
>>>   tests/qapi-schema/doc-bad-whitespaces.out  |  0
>>>   tests/qapi-schema/meson.build              |  2 +
>>>   8 files changed, 68 insertions(+), 1 deletion(-)
>>>   create mode 100644 tests/qapi-schema/doc-bad-long-line.err
>>>   create mode 100644 tests/qapi-schema/doc-bad-long-line.json
>>>   create mode 100644 tests/qapi-schema/doc-bad-long-line.out
>>>   create mode 100644 tests/qapi-schema/doc-bad-whitespaces.err
>>>   create mode 100644 tests/qapi-schema/doc-bad-whitespaces.json
>>>   create mode 100644 tests/qapi-schema/doc-bad-whitespaces.out
>>>
>>> diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py
>>> index 9fbf80a541..ffb149850d 100644
>>> --- a/scripts/qapi/parser.py
>>> +++ b/scripts/qapi/parser.py
>>> @@ -108,6 +108,11 @@ def __init__(self,
>>>           self.exprs: List[QAPIExpression] = []
>>>           self.docs: List[QAPIDoc] = []
>>>   
>>> +        # State for tracking qmp-example blocks and simple
>>> +        # :: literal blocks.
>>> +        self._literal_mode = False
>>> +        self._literal_mode_indent = 0
>>> +
>>>           # Showtime!
>>>           self._parse()
>>>   
>>> @@ -423,12 +428,57 @@ def get_doc_line(self) -> Optional[str]:
>>>               if self.val != '##':
>>>                   raise QAPIParseError(
>>>                       self, "junk after '##' at end of documentation comment")
>>> +            self._literal_mode = False
>>>               return None
>>>           if self.val == '#':
>>>               return ''
>>>           if self.val[1] != ' ':
>>>               raise QAPIParseError(self, "missing space after #")
>>> -        return self.val[2:].rstrip()
>>> +
>>> +        line = self.val[2:].rstrip()
>>> +
>>> +        if re.match(r'(\.\. +qmp-example)? *::$', line):
>> 
>> After a closer reading of ReST docs and some testing: this isn't quite
>> right, although it works okay for what we have.
>> 
>> A directive's '::' need not be at the end of a line.  The regexp fails
>> to match a qmp-example directive with text after '::'.  No such
>> directives exist right now.
>> 
>> A literal block starts after a '::' at the end of a paragraph,
>> i.e. after '::' and a blank line.  The regexp only matches '::' on its
>> own line, not at the end of a line of text.  It matches it even when
>> it's not followed by a blank line.
>> 
>> In review of v4, I claimed we don't use the contracted form "text::".
>> Not true.  For instance, in block-core.json:
>> 
>>     # @bins: list of io request counts corresponding to histogram
>>     #     intervals, one more element than @boundaries has.  For the
>>     #     example above, @bins may be something like [3, 1, 5, 2], and
>>     #     corresponding histogram looks like::
>>     #
>>     #        5|           *
>>     #        4|           *
>>     #        3| *         *
>>     #        2| *         *    *
>>     #        1| *    *    *    *
>>     #         +------------------
>>     #             10   50   100
>>     #
>
> Ow, my old histograms)

Haha!

>>     # Since: 4.0
>>     ##
>> 
>> The literal block starts after "like::" and ends before "Since:'.
>> 
>>> +            self._literal_mode = True
>>> +            self._literal_mode_indent = 0
>>> +        elif self._literal_mode and line:
>>> +            indent = re.match(r'^ *', line).end()
>>> +            if self._literal_mode_indent == 0:
>>> +                self._literal_mode_indent = indent
>>> +            elif indent < self._literal_mode_indent:
>>> +                # ReST directives stop at decreasing indentation
>>> +                self._literal_mode = False
>> 
>> This isn't quite right, either.  We need to stop when indentation of
>> non-blank lines drops below the indentation of the line containing the
>> '::'.
>> 
>> Perhaps it's easier for both of us if I fix this on top.  Thoughts?
>
> No objections, good for me!

I can merge this with a brief note, like so:

    Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>

    The detection of example and literal blocks isn't quite correct, but
    it works well enough, and we can improve on top.

    Reviewed-by: Markus Armbruster <armbru@redhat.com>

>>> +
>>> +        if not self._literal_mode:
>>> +            self._validate_doc_line_format(line)
>>> +
>>> +        return line
>>> +
>>> +    def _validate_doc_line_format(self, line: str) -> None:
>>> +        """
>>> +        Validate documentation format rules for a single line:
>>> +        1. Lines should not exceed 70 columns
>>> +        2. Sentences should be separated by two spaces
>>> +        """
>>> +        full_line_length = len(line) + 2  # "# " = 2 characters
>>> +        if full_line_length > 70:
>>> +            # Skip URL lines - they can't be broken
>>> +            if re.match(r' *(https?|ftp)://[^ ]*$', line):
>>> +                pass
>>> +            else:
>>> +                raise QAPIParseError(
>>> +                    self, "documentation line exceeds 70 columns"
>>> +                )
>>> +
>>> +        single_space_pattern = r'(\be\.g\.|^ *\d\.|([.!?])) [A-Z0-9(]'
>>> +        for m in list(re.finditer(single_space_pattern, line)):
>>> +            if not m.group(2):
>>> +                continue
>>> +            # HACK so the error message points to the offending spot
>>> +            self.pos = self.line_pos + 2 + m.start(2) + 1
>>> +            raise QAPIParseError(
>>> +                self, "Use two spaces between sentences\n"
>>> +                "If this not the end of a sentence, please report the bug",
>>> +            )
>>>   
>>>       @staticmethod
>>>       def _match_at_name_colon(string: str) -> Optional[Match[str]]:
>>> diff --git a/tests/qapi-schema/doc-bad-long-line.err b/tests/qapi-schema/doc-bad-long-line.err
>>> new file mode 100644
>>> index 0000000000..611a3b1fef
>>> --- /dev/null
>>> +++ b/tests/qapi-schema/doc-bad-long-line.err
>>> @@ -0,0 +1 @@
>>> +doc-bad-long-line.json:4:1: documentation line exceeds 70 columns
>>> diff --git a/tests/qapi-schema/doc-bad-long-line.json b/tests/qapi-schema/doc-bad-long-line.json
>>> new file mode 100644
>>> index 0000000000..d7f887694d
>>> --- /dev/null
>>> +++ b/tests/qapi-schema/doc-bad-long-line.json
>>> @@ -0,0 +1,6 @@
>>> +##
>>> +# @foo:
>>> +#
>>> +# This line has exactly 71 characters, including spaces and punctuation!
>> 
>> Really?
>
> Oh, it's 72 characters actually! AI tricked me. Didn't I check it out?
>
> Maybe:
>
> # This line has exactly 71 chars, including the leading hash and space.

Works!

>>> +##
>>> +{ 'command': 'foo' }
>>> diff --git a/tests/qapi-schema/doc-bad-long-line.out b/tests/qapi-schema/doc-bad-long-line.out
>>> new file mode 100644
>>> index 0000000000..e69de29bb2
>>> diff --git a/tests/qapi-schema/doc-bad-whitespaces.err b/tests/qapi-schema/doc-bad-whitespaces.err
>>> new file mode 100644
>>> index 0000000000..5cca1954c0
>>> --- /dev/null
>>> +++ b/tests/qapi-schema/doc-bad-whitespaces.err
>>> @@ -0,0 +1,2 @@
>>> +doc-bad-whitespaces.json:4:48: Use two spaces between sentences
>>> +If this not the end of a sentence, please report the bug
>>> diff --git a/tests/qapi-schema/doc-bad-whitespaces.json b/tests/qapi-schema/doc-bad-whitespaces.json
>>> new file mode 100644
>>> index 0000000000..b0c318c670
>>> --- /dev/null
>>> +++ b/tests/qapi-schema/doc-bad-whitespaces.json
>>> @@ -0,0 +1,6 @@
>>> +##
>>> +# @foo:
>>> +#
>>> +# Sentences should be split by two whitespaces. But here is only one.
>> 
>> two spaces
>> 
>>> +##
>>> +{ 'command': 'foo' }
>>> diff --git a/tests/qapi-schema/doc-bad-whitespaces.out b/tests/qapi-schema/doc-bad-whitespaces.out
>>> new file mode 100644
>>> index 0000000000..e69de29bb2
>>> diff --git a/tests/qapi-schema/meson.build b/tests/qapi-schema/meson.build
>>> index c47025d16d..b24b27db21 100644
>>> --- a/tests/qapi-schema/meson.build
>>> +++ b/tests/qapi-schema/meson.build
>>> @@ -61,8 +61,10 @@ schemas = [
>>>     'doc-bad-event-arg.json',
>>>     'doc-bad-feature.json',
>>>     'doc-bad-indent.json',
>>> +  'doc-bad-long-line.json',
>>>     'doc-bad-symbol.json',
>>>     'doc-bad-union-member.json',
>>> +  'doc-bad-whitespaces.json',
>>>     'doc-before-include.json',
>>>     'doc-before-pragma.json',
>>>     'doc-duplicate-features.json',

Thanks!

Re: [PATCH v5] qapi: Add documentation format validation

[Vladimir Sementsov-Ogievskiy]

On 04.11.25 14:46, Markus Armbruster wrote:
> Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:
> 
>> On 04.11.25 13:07, Markus Armbruster wrote:
>>> Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:
>>>
>>>> Add explicit validation for QAPI documentation formatting rules:
>>>>
>>>> 1. Lines must not exceed 70 columns in width (including '# ' prefix)
>>>> 2. Sentences must be separated by two spaces
>>>>
>>>> Example sections and literal :: blocks (seldom case) are excluded, we
>>>> don't require them to be <= 70, that would be too restrictive. Anyway,
>>>> they share common 80-columns recommendations (not requirements).
>>>>
>>>> Add two simple tests, illustrating the change.
>>>>
>>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
>>>> ---
>>>>
>>>> Hi all!
>>>>
>>>> v5: - break "literal" block at any decreasing the indent,
>>>>         not only at no-indent
>>>>       - add two simple tests
>>>>
>>>> This is based on
>>>> [PATCH 0/8] A QAPI schema doc markup fix, and style cleanup
>>>> Based-on: <20251031094751.2817932-1-armbru@redhat.com>
>>>>
>>>>    scripts/qapi/parser.py                     | 52 +++++++++++++++++++++-
>>>>    tests/qapi-schema/doc-bad-long-line.err    |  1 +
>>>>    tests/qapi-schema/doc-bad-long-line.json   |  6 +++
>>>>    tests/qapi-schema/doc-bad-long-line.out    |  0
>>>>    tests/qapi-schema/doc-bad-whitespaces.err  |  2 +
>>>>    tests/qapi-schema/doc-bad-whitespaces.json |  6 +++
>>>>    tests/qapi-schema/doc-bad-whitespaces.out  |  0
>>>>    tests/qapi-schema/meson.build              |  2 +
>>>>    8 files changed, 68 insertions(+), 1 deletion(-)
>>>>    create mode 100644 tests/qapi-schema/doc-bad-long-line.err
>>>>    create mode 100644 tests/qapi-schema/doc-bad-long-line.json
>>>>    create mode 100644 tests/qapi-schema/doc-bad-long-line.out
>>>>    create mode 100644 tests/qapi-schema/doc-bad-whitespaces.err
>>>>    create mode 100644 tests/qapi-schema/doc-bad-whitespaces.json
>>>>    create mode 100644 tests/qapi-schema/doc-bad-whitespaces.out
>>>>
>>>> diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py
>>>> index 9fbf80a541..ffb149850d 100644
>>>> --- a/scripts/qapi/parser.py
>>>> +++ b/scripts/qapi/parser.py
>>>> @@ -108,6 +108,11 @@ def __init__(self,
>>>>            self.exprs: List[QAPIExpression] = []
>>>>            self.docs: List[QAPIDoc] = []
>>>>    
>>>> +        # State for tracking qmp-example blocks and simple
>>>> +        # :: literal blocks.
>>>> +        self._literal_mode = False
>>>> +        self._literal_mode_indent = 0
>>>> +
>>>>            # Showtime!
>>>>            self._parse()
>>>>    
>>>> @@ -423,12 +428,57 @@ def get_doc_line(self) -> Optional[str]:
>>>>                if self.val != '##':
>>>>                    raise QAPIParseError(
>>>>                        self, "junk after '##' at end of documentation comment")
>>>> +            self._literal_mode = False
>>>>                return None
>>>>            if self.val == '#':
>>>>                return ''
>>>>            if self.val[1] != ' ':
>>>>                raise QAPIParseError(self, "missing space after #")
>>>> -        return self.val[2:].rstrip()
>>>> +
>>>> +        line = self.val[2:].rstrip()
>>>> +
>>>> +        if re.match(r'(\.\. +qmp-example)? *::$', line):
>>>
>>> After a closer reading of ReST docs and some testing: this isn't quite
>>> right, although it works okay for what we have.
>>>
>>> A directive's '::' need not be at the end of a line.  The regexp fails
>>> to match a qmp-example directive with text after '::'.  No such
>>> directives exist right now.
>>>
>>> A literal block starts after a '::' at the end of a paragraph,
>>> i.e. after '::' and a blank line.  The regexp only matches '::' on its
>>> own line, not at the end of a line of text.  It matches it even when
>>> it's not followed by a blank line.
>>>
>>> In review of v4, I claimed we don't use the contracted form "text::".
>>> Not true.  For instance, in block-core.json:
>>>
>>>      # @bins: list of io request counts corresponding to histogram
>>>      #     intervals, one more element than @boundaries has.  For the
>>>      #     example above, @bins may be something like [3, 1, 5, 2], and
>>>      #     corresponding histogram looks like::
>>>      #
>>>      #        5|           *
>>>      #        4|           *
>>>      #        3| *         *
>>>      #        2| *         *    *
>>>      #        1| *    *    *    *
>>>      #         +------------------
>>>      #             10   50   100
>>>      #
>>
>> Ow, my old histograms)
> 
> Haha!
> 
>>>      # Since: 4.0
>>>      ##
>>>
>>> The literal block starts after "like::" and ends before "Since:'.
>>>
>>>> +            self._literal_mode = True
>>>> +            self._literal_mode_indent = 0
>>>> +        elif self._literal_mode and line:
>>>> +            indent = re.match(r'^ *', line).end()
>>>> +            if self._literal_mode_indent == 0:
>>>> +                self._literal_mode_indent = indent
>>>> +            elif indent < self._literal_mode_indent:
>>>> +                # ReST directives stop at decreasing indentation
>>>> +                self._literal_mode = False
>>>
>>> This isn't quite right, either.  We need to stop when indentation of
>>> non-blank lines drops below the indentation of the line containing the
>>> '::'.
>>>
>>> Perhaps it's easier for both of us if I fix this on top.  Thoughts?
>>
>> No objections, good for me!
> 
> I can merge this with a brief note, like so:
> 
>      Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> 
>      The detection of example and literal blocks isn't quite correct, but
>      it works well enough, and we can improve on top.
> 
>      Reviewed-by: Markus Armbruster <armbru@redhat.com>
> 

Ok, thanks!

>>>> +
>>>> +        if not self._literal_mode:
>>>> +            self._validate_doc_line_format(line)
>>>> +
>>>> +        return line
>>>> +
>>>> +    def _validate_doc_line_format(self, line: str) -> None:
>>>> +        """
>>>> +        Validate documentation format rules for a single line:
>>>> +        1. Lines should not exceed 70 columns
>>>> +        2. Sentences should be separated by two spaces
>>>> +        """
>>>> +        full_line_length = len(line) + 2  # "# " = 2 characters
>>>> +        if full_line_length > 70:
>>>> +            # Skip URL lines - they can't be broken
>>>> +            if re.match(r' *(https?|ftp)://[^ ]*$', line):
>>>> +                pass
>>>> +            else:
>>>> +                raise QAPIParseError(
>>>> +                    self, "documentation line exceeds 70 columns"
>>>> +                )
>>>> +
>>>> +        single_space_pattern = r'(\be\.g\.|^ *\d\.|([.!?])) [A-Z0-9(]'
>>>> +        for m in list(re.finditer(single_space_pattern, line)):
>>>> +            if not m.group(2):
>>>> +                continue
>>>> +            # HACK so the error message points to the offending spot
>>>> +            self.pos = self.line_pos + 2 + m.start(2) + 1
>>>> +            raise QAPIParseError(
>>>> +                self, "Use two spaces between sentences\n"
>>>> +                "If this not the end of a sentence, please report the bug",
>>>> +            )
>>>>    
>>>>        @staticmethod
>>>>        def _match_at_name_colon(string: str) -> Optional[Match[str]]:
>>>> diff --git a/tests/qapi-schema/doc-bad-long-line.err b/tests/qapi-schema/doc-bad-long-line.err
>>>> new file mode 100644
>>>> index 0000000000..611a3b1fef
>>>> --- /dev/null
>>>> +++ b/tests/qapi-schema/doc-bad-long-line.err
>>>> @@ -0,0 +1 @@
>>>> +doc-bad-long-line.json:4:1: documentation line exceeds 70 columns
>>>> diff --git a/tests/qapi-schema/doc-bad-long-line.json b/tests/qapi-schema/doc-bad-long-line.json
>>>> new file mode 100644
>>>> index 0000000000..d7f887694d
>>>> --- /dev/null
>>>> +++ b/tests/qapi-schema/doc-bad-long-line.json
>>>> @@ -0,0 +1,6 @@
>>>> +##
>>>> +# @foo:
>>>> +#
>>>> +# This line has exactly 71 characters, including spaces and punctuation!
>>>
>>> Really?
>>
>> Oh, it's 72 characters actually! AI tricked me. Didn't I check it out?
>>
>> Maybe:
>>
>> # This line has exactly 71 chars, including the leading hash and space.
> 
> Works!
> 
>>>> +##
>>>> +{ 'command': 'foo' }
>>>> diff --git a/tests/qapi-schema/doc-bad-long-line.out b/tests/qapi-schema/doc-bad-long-line.out
>>>> new file mode 100644
>>>> index 0000000000..e69de29bb2
>>>> diff --git a/tests/qapi-schema/doc-bad-whitespaces.err b/tests/qapi-schema/doc-bad-whitespaces.err
>>>> new file mode 100644
>>>> index 0000000000..5cca1954c0
>>>> --- /dev/null
>>>> +++ b/tests/qapi-schema/doc-bad-whitespaces.err
>>>> @@ -0,0 +1,2 @@
>>>> +doc-bad-whitespaces.json:4:48: Use two spaces between sentences
>>>> +If this not the end of a sentence, please report the bug
>>>> diff --git a/tests/qapi-schema/doc-bad-whitespaces.json b/tests/qapi-schema/doc-bad-whitespaces.json
>>>> new file mode 100644
>>>> index 0000000000..b0c318c670
>>>> --- /dev/null
>>>> +++ b/tests/qapi-schema/doc-bad-whitespaces.json
>>>> @@ -0,0 +1,6 @@
>>>> +##
>>>> +# @foo:
>>>> +#
>>>> +# Sentences should be split by two whitespaces. But here is only one.
>>>
>>> two spaces
>>>
>>>> +##
>>>> +{ 'command': 'foo' }
>>>> diff --git a/tests/qapi-schema/doc-bad-whitespaces.out b/tests/qapi-schema/doc-bad-whitespaces.out
>>>> new file mode 100644
>>>> index 0000000000..e69de29bb2
>>>> diff --git a/tests/qapi-schema/meson.build b/tests/qapi-schema/meson.build
>>>> index c47025d16d..b24b27db21 100644
>>>> --- a/tests/qapi-schema/meson.build
>>>> +++ b/tests/qapi-schema/meson.build
>>>> @@ -61,8 +61,10 @@ schemas = [
>>>>      'doc-bad-event-arg.json',
>>>>      'doc-bad-feature.json',
>>>>      'doc-bad-indent.json',
>>>> +  'doc-bad-long-line.json',
>>>>      'doc-bad-symbol.json',
>>>>      'doc-bad-union-member.json',
>>>> +  'doc-bad-whitespaces.json',
>>>>      'doc-before-include.json',
>>>>      'doc-before-pragma.json',
>>>>      'doc-duplicate-features.json',
> 
> Thanks!
> 


-- 
Best regards,
Vladimir

Re: [PATCH v5] qapi: Add documentation format validation

[Markus Armbruster]

Queued, thanks!