John Snow <js...@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 <js...@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:
