If we remove the legacy parser, the doc-good.json formatting begins to
fail because the body text is appended directly after the field list
entry, which is invalid rST syntax.
Without this change, we see this error:
/home/jsnow/src/qemu/docs/../tests/qapi-schema/doc-good.json:169:
WARNING: Field list ends without a blank line; unexpected
unindent. [docutils]
And this intermediate rST source:
tests/qapi-schema/doc-good.json:0167 | :error:
tests/qapi-schema/doc-good.json:0168 | some
With this patch applied, we instead generate this source:
tests/qapi-schema/doc-good.json:0167 | :error:
tests/qapi-schema/doc-good.json:0168 | - some
which compiles successfully.
Signed-off-by: John Snow <jsnow@redhat.com>
---
tests/qapi-schema/doc-good.json | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tests/qapi-schema/doc-good.json b/tests/qapi-schema/doc-good.json
index 14b808f9090..6dcde8fd7e8 100644
--- a/tests/qapi-schema/doc-good.json
+++ b/tests/qapi-schema/doc-good.json
@@ -165,7 +165,8 @@
#
# Returns: @Object
#
-# Errors: some
+# Errors:
+# - some
#
# TODO: frobnicate
#
--
2.48.1John Snow <jsnow@redhat.com> writes:
> If we remove the legacy parser, the doc-good.json formatting begins to
"parser"? You mean docs/sphinx/qapidoc_legacy.py, don't you?
> fail because the body text is appended directly after the field list
> entry, which is invalid rST syntax.
We've been running the test suite with the legacy doc generator.
Unwise; we should've switched to the new one right away.
> Without this change, we see this error:
>
> /home/jsnow/src/qemu/docs/../tests/qapi-schema/doc-good.json:169:
> WARNING: Field list ends without a blank line; unexpected
> unindent. [docutils]
The reporting is less than helpful.
> And this intermediate rST source:
>
> tests/qapi-schema/doc-good.json:0167 | :error:
> tests/qapi-schema/doc-good.json:0168 | some
>
> With this patch applied, we instead generate this source:
>
> tests/qapi-schema/doc-good.json:0167 | :error:
> tests/qapi-schema/doc-good.json:0168 | - some
>
> which compiles successfully.
Hmm.
As far as I can tell, the problem is lack of indentation[*].
By convention, the contents of an Errors: section is a list.
docs/devel/qapi-code-gen.rst:
"Errors" sections should be formatted as an rST list, each entry
detailing a relevant error condition. For example::
# Errors:
# - If @device does not exist, DeviceNotFound
# - Any other error returns a GenericError.
This test case is the only instance of something else.
It's just a convention, though.
Your change to the positive test case makes some sense all the same; it
should cover how we want the thing to be used.
What I don't like is how the new doc generator fails when we fail to
adhere to the convention.
Here's docs/devel/qapi-code-gen.rst on tagged sections:
A tagged section begins with a paragraph that starts with one of the
following words: "Since:", "Returns:", "Errors:", "TODO:". It ends with
the start of a new section.
The second and subsequent lines of tagged sections must be indented
like this::
# TODO: Ut enim ad minim veniam, quis nostrud exercitation ullamco
# laboris nisi ut aliquip ex ea commodo consequat.
#
# Duis aute irure dolor in reprehenderit in voluptate velit esse
# cillum dolore eu fugiat nulla pariatur.
This tells us that
# Errors: some
and
# Errors:
# some
and
# Errors: some
# more
should all work, just like for any other tag. However, only the second
one works in my testing. With qapidoc_legacy.py, all three work.
We can make Errors: unlike the other tags. But it needs to be done
properly, i.e. in scripts/qapi/parser.py (for decent error reporting),
and documented in docs/devel/qapi-code-gen.rst.
Keeping the QAPI domain accept what the generator generates might be
easier.
Thoughts?
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
> tests/qapi-schema/doc-good.json | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/tests/qapi-schema/doc-good.json b/tests/qapi-schema/doc-good.json
> index 14b808f9090..6dcde8fd7e8 100644
> --- a/tests/qapi-schema/doc-good.json
> +++ b/tests/qapi-schema/doc-good.json
> @@ -165,7 +165,8 @@
> #
> # Returns: @Object
> #
> -# Errors: some
> +# Errors:
> +# - some
> #
> # TODO: frobnicate
> #
Fails "make check". Fixup appended.
[*] Evidence:
# Errors:
# - some
which expands into
:error:
- some
and
# Errors:
# some
which expands into
:error:
some
both work.
docs/devel/qapi-domain.rst:
``:error:``
-----------
Document the error condition(s) of a QAPI command.
:availability: This field list is only available in the body of the
Command directive.
--> :syntax: ``:error: Lorem ipsum dolor sit amet ...``
:type: `sphinx.util.docfields.Field
<https://pydoc.dev/sphinx/latest/sphinx.util.docfields.Field.html?private=1>`_
The format of the :errors: field list description is free-form rST. The
alternative spelling ":errors:" is also permitted, but strictly
analogous.
Example::
.. qapi:command:: block-job-set-speed
:since: 1.1
Set maximum speed for a background block operation.
This command can only be issued when there is an active block job.
Throttling can be disabled by setting the speed to 0.
:arg string device: The job identifier. This used to be a device
name (hence the name of the parameter), but since QEMU 2.7 it
can have other values.
:arg int speed: the maximum speed, in bytes per second, or 0 for
unlimited. Defaults to 0.
--> :error:
--> - If no background operation is active on this device,
--> DeviceNotActive
This makes me expect
:error: some
also works. However, the obvious
# Errors: some
produces
:error:
some
which doesn't work.
diff --git a/tests/qapi-schema/doc-good.out b/tests/qapi-schema/doc-good.out
index dc8352eed4..3711cf5480 100644
--- a/tests/qapi-schema/doc-good.out
+++ b/tests/qapi-schema/doc-good.out
@@ -176,7 +176,7 @@ another feature
section=Returns
@Object
section=Errors
-some
+ - some
section=Todo
frobnicate
section=Plain
diff --git a/tests/qapi-schema/doc-good.txt b/tests/qapi-schema/doc-good.txt
index 17a1d56ef1..e54cc95f4a 100644
--- a/tests/qapi-schema/doc-good.txt
+++ b/tests/qapi-schema/doc-good.txt
@@ -207,7 +207,7 @@ Returns
Errors
~~~~~~
-some
+* some
Notes:
On Mon, Jun 16, 2025 at 7:36 AM Markus Armbruster <armbru@redhat.com> wrote: > John Snow <jsnow@redhat.com> writes: > > > If we remove the legacy parser, the doc-good.json formatting begins to > > "parser"? You mean docs/sphinx/qapidoc_legacy.py, don't you? > Mmm... yes, I'm conflating the purpose of the series (removing the legacy freeform doc parser) with what necessitated this change (switching to the new doc *generator*) Wiggly-brained, wiggly-mouthed. > > > fail because the body text is appended directly after the field list > > entry, which is invalid rST syntax. > > We've been running the test suite with the legacy doc generator. > Unwise; we should've switched to the new one right away. > Oops O:-) > > > Without this change, we see this error: > > > > /home/jsnow/src/qemu/docs/../tests/qapi-schema/doc-good.json:169: > > WARNING: Field list ends without a blank line; unexpected > > unindent. [docutils] > > The reporting is less than helpful. > > > And this intermediate rST source: > > > > tests/qapi-schema/doc-good.json:0167 | :error: > > tests/qapi-schema/doc-good.json:0168 | some > > > > With this patch applied, we instead generate this source: > > > > tests/qapi-schema/doc-good.json:0167 | :error: > > tests/qapi-schema/doc-good.json:0168 | - some > > > > which compiles successfully. > > Hmm. > > As far as I can tell, the problem is lack of indentation[*]. > > By convention, the contents of an Errors: section is a list. > docs/devel/qapi-code-gen.rst: > > "Errors" sections should be formatted as an rST list, each entry > detailing a relevant error condition. For example:: > > # Errors: > # - If @device does not exist, DeviceNotFound > # - Any other error returns a GenericError. > > This test case is the only instance of something else. > > It's just a convention, though. > > Your change to the positive test case makes some sense all the same; it > should cover how we want the thing to be used. > > What I don't like is how the new doc generator fails when we fail to > adhere to the convention. > I agree... it should probably generate ":error: some" instead, without the newline. Bad, but valid. > > Here's docs/devel/qapi-code-gen.rst on tagged sections: > > A tagged section begins with a paragraph that starts with one of the > following words: "Since:", "Returns:", "Errors:", "TODO:". It ends > with > the start of a new section. > > The second and subsequent lines of tagged sections must be indented > like this:: > > # TODO: Ut enim ad minim veniam, quis nostrud exercitation ullamco > # laboris nisi ut aliquip ex ea commodo consequat. > # > # Duis aute irure dolor in reprehenderit in voluptate velit esse > # cillum dolore eu fugiat nulla pariatur. > > This tells us that > > # Errors: some > > and > > # Errors: > # some > > and > > # Errors: some > # more > > should all work, just like for any other tag. However, only the second > one works in my testing. With qapidoc_legacy.py, all three work. > > We can make Errors: unlike the other tags. But it needs to be done > properly, i.e. in scripts/qapi/parser.py (for decent error reporting), > and documented in docs/devel/qapi-code-gen.rst. > > Keeping the QAPI domain accept what the generator generates might be > easier. > > Thoughts? > I try not to have any as often as I can. In seriousness, I need a good few minutes with the generator to understand why this behaves strangely. Might need to switch to a different parser routine in parser.py, or I might need to adjust the qapidoc generator. Not entirely sure what's precisely wrong... If I can find something that's both easy and clean I'll do that instead. Probably a newline needs to be preserved in the source somewhere, and then a newline needs to be *not* added to the generator. Something like that. We can discuss turning our de-facto standard into an actual syntactical requirement later; I think there might be some benefit to allowing multiple errors sections that each become their own ":error:" info field list entry, but I am not confident on that and am not ready to dive into it just yet. I think it's definitely easier on the sphinx side but it might not be easier on the QAPI side. We'll see... > > > > > Signed-off-by: John Snow <jsnow@redhat.com> > > --- > > tests/qapi-schema/doc-good.json | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/tests/qapi-schema/doc-good.json > b/tests/qapi-schema/doc-good.json > > index 14b808f9090..6dcde8fd7e8 100644 > > --- a/tests/qapi-schema/doc-good.json > > +++ b/tests/qapi-schema/doc-good.json > > @@ -165,7 +165,8 @@ > > # > > # Returns: @Object > > # > > -# Errors: some > > +# Errors: > > +# - some > > # > > # TODO: frobnicate > > # > > Fails "make check". Fixup appended. > Strange. How'd I find this issue and fix it if I wasn't running the tests? Uhm, sorry. Sloppy of me. > > > > [*] Evidence: > > # Errors: > # - some > > which expands into > > :error: > - some > > and > > # Errors: > # some > > which expands into > > :error: > some > > both work. > > docs/devel/qapi-domain.rst: > > ``:error:`` > ----------- > > Document the error condition(s) of a QAPI command. > > :availability: This field list is only available in the body of the > Command directive. > --> :syntax: ``:error: Lorem ipsum dolor sit amet ...`` > :type: `sphinx.util.docfields.Field > < > https://pydoc.dev/sphinx/latest/sphinx.util.docfields.Field.html?private=1 > >`_ > > The format of the :errors: field list description is free-form rST. The > alternative spelling ":errors:" is also permitted, but strictly > analogous. > > Example:: > > .. qapi:command:: block-job-set-speed > :since: 1.1 > > Set maximum speed for a background block operation. > > This command can only be issued when there is an active block > job. > > Throttling can be disabled by setting the speed to 0. > > :arg string device: The job identifier. This used to be a device > name (hence the name of the parameter), but since QEMU 2.7 it > can have other values. > :arg int speed: the maximum speed, in bytes per second, or 0 for > unlimited. Defaults to 0. > --> :error: > --> - If no background operation is active on this device, > --> DeviceNotActive > > This makes me expect > > :error: some > > also works. However, the obvious > > # Errors: some > > produces > > :error: > some > > which doesn't work. > > > diff --git a/tests/qapi-schema/doc-good.out > b/tests/qapi-schema/doc-good.out > index dc8352eed4..3711cf5480 100644 > --- a/tests/qapi-schema/doc-good.out > +++ b/tests/qapi-schema/doc-good.out > @@ -176,7 +176,7 @@ another feature > section=Returns > @Object > section=Errors > -some > + - some > section=Todo > frobnicate > section=Plain > diff --git a/tests/qapi-schema/doc-good.txt > b/tests/qapi-schema/doc-good.txt > index 17a1d56ef1..e54cc95f4a 100644 > --- a/tests/qapi-schema/doc-good.txt > +++ b/tests/qapi-schema/doc-good.txt > @@ -207,7 +207,7 @@ Returns > Errors > ~~~~~~ > > -some > +* some > > Notes: > > >
© 2016 - 2025 Red Hat, Inc.