Re: [PATCH v2 1/3] docs: fix errors formatting in tests/qapi-schema/doc-good

[Markus Armbruster <armbru@redhat.com>]

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?

> 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: