Vladimir Sementsov-Ogievskiy <[email protected]> writes:
> On 04.11.25 13:07, Markus Armbruster wrote:
>> Vladimir Sementsov-Ogievskiy <[email protected]> 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 <[email protected]>
>>> ---
>>>
>>> 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: <[email protected]>
>>>
>>> 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 <[email protected]>
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 <[email protected]>
>>> +
>>> + 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!
