John Snow <js...@redhat.com> writes:
> Leading and trailing whitespace are now discarded, addressing the FIXME
> comment. A new error is raised to detect this accidental case.
>
> Parsing for args sections is left alone here; the 'name' variable is
> moved into the only block where it is used.
>
> Signed-off-by: John Snow <js...@redhat.com>
>
> ---
>
> Tangentially related to delinting in that removing 'FIXME' comments is a
> goal for pylint. My goal is to allow 'TODO' to be checked in, but
> 'FIXME' should be fixed prior to inclusion.
>
> Arbitrary, but that's life for you.
>
> Signed-off-by: John Snow <js...@redhat.com>
> ---
> scripts/qapi/parser.py | 13 ++++++++-----
> tests/qapi-schema/doc-whitespace-leading-symbol.err | 1 +
> .../qapi-schema/doc-whitespace-leading-symbol.json | 6 ++++++
> tests/qapi-schema/doc-whitespace-leading-symbol.out | 0
> .../qapi-schema/doc-whitespace-trailing-symbol.err | 1 +
> .../qapi-schema/doc-whitespace-trailing-symbol.json | 6 ++++++
> .../qapi-schema/doc-whitespace-trailing-symbol.out | 0
> tests/qapi-schema/meson.build | 2 ++
> 8 files changed, 24 insertions(+), 5 deletions(-)
> create mode 100644 tests/qapi-schema/doc-whitespace-leading-symbol.err
> create mode 100644 tests/qapi-schema/doc-whitespace-leading-symbol.json
> create mode 100644 tests/qapi-schema/doc-whitespace-leading-symbol.out
> create mode 100644 tests/qapi-schema/doc-whitespace-trailing-symbol.err
> create mode 100644 tests/qapi-schema/doc-whitespace-trailing-symbol.json
> create mode 100644 tests/qapi-schema/doc-whitespace-trailing-symbol.out
>
> diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py
> index bfd2dbfd9a2..2f93a752f66 100644
> --- a/scripts/qapi/parser.py
> +++ b/scripts/qapi/parser.py
> @@ -549,18 +549,21 @@ def _append_body_line(self, line):
>
> Else, append the line to the current section.
> """
> - name = line.split(' ', 1)[0]
> - # FIXME not nice: things like '# @foo:' and '# @foo: ' aren't
> - # recognized, and get silently treated as ordinary text
> - if not self.symbol and not self.body.text and line.startswith('@'):
> - if not line.endswith(':'):
> + stripped = line.strip()
> +
> + if not self.symbol and not self.body.text and
> stripped.startswith('@'):
> + if not stripped.endswith(':'):
> raise QAPIParseError(self._parser, "line should end with
> ':'")
> + if not stripped == line:
> + raise QAPIParseError(
> + self._parser, "extra whitespace around symbol
> declaration")
This rejects both leading and trailing whitespace. Rejecting leading
whitespace is good. Rejecting trailing whitespace feels a bit pedantic,
and it might not extend to the related case I'll point out below.
Have you considered a regexp instead? Say
match = re.match(r'(\s*)@([^:]*)(:?)(\s*)(.*)$', line)
Then match.group(n) is
n=1 leading whitespace, if any
n=2 symbol
n=3 trailing colon, if any
n=4 trailing whitespace, if any
n=5 trailing text, if any
Omit the subgroups you don't need.
> self.symbol = line[1:-1]
> # FIXME invalid names other than the empty string aren't flagged
> if not self.symbol:
> raise QAPIParseError(self._parser, "invalid name")
> elif self.symbol:
> # This is a definition documentation block
> + name = line.split(' ', 1)[0]
> if name.startswith('@') and name.endswith(':'):
> self._append_line = self._append_args_line
> self._append_args_line(line)
Same issue here, and in _append_args_line(). To reproduce, I hacked up
doc-good.json like so
diff --git a/tests/qapi-schema/doc-good.json
b/tests/qapi-schema/doc-good.json
index 86dc25d2bd..977fcbad48 100644
--- a/tests/qapi-schema/doc-good.json
+++ b/tests/qapi-schema/doc-good.json
@@ -133,7 +133,7 @@
##
# @cmd:
#
-# @arg1: the first argument
+# @arg1: the first argument
#
# @arg2: the second
# argument
and got
$ PYTHONPATH=/work/armbru/qemu/scripts python3
/work/armbru/qemu/tests/qapi-schema/test-qapi.py -d tests/qapi-schema
doc-good.json
doc-good FAIL
--- tests/qapi-schema/doc-good.out
+++
@@ -149,12 +149,12 @@
== Another subsection
doc symbol=cmd
body=
-
- arg=arg1
-the first argument
+@arg1: the first argument
arg=arg2
the second
argument
+ arg=arg1
+
arg=arg3
feature=cmd-feat1
[...]
