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
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
Queued, thanks!
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',
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
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!
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
© 2016 - 2025 Red Hat, Inc.