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
#
# 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',
