Re: [PATCH 8/8] qapi: remove "Example" doc section

[Markus Armbruster]

John Snow <jsnow@redhat.com> writes:

> On Tue, Jul 9, 2024 at 6:52 AM Markus Armbruster <armbru@redhat.com> wrote:
>
>> John Snow <jsnow@redhat.com> writes:
>>
>> > Fully eliminate the "Example" sections in QAPI doc blocks now that they
>> > have all been converted to arbitrary rST syntax using the
>> > ".. qmp-example::" directive. Update tests to match.
>> >
>> > Migrating to the new syntax
>> > ---------------------------
>> >
>> > The old "Example:" or "Examples:" section syntax is now caught as an
>> > error, but "Example::" is stil permitted as explicit rST syntax for an
>> > un-lexed, generic preformatted text block.
>> >
>> > ('Example' is not special in this case, any sentence that ends with "::"
>> > will start an indented code block in rST.)
>> >
>> > Arbitrary rST for Examples is now possible, but it's strongly
>> > recommended that documentation authors use the ".. qmp-example::"
>> > directive for consistent visual formatting in rendered HTML docs. The
>> > ":title:" directive option may be used to add extra information into the
>> > title bar for the example. The ":annotated:" option can be used to write
>> > arbitrary rST instead, with nested "::" blocks applying QMP formatting
>> > where desired.
>> >
>> > Other choices available are ".. code-block:: QMP" which will not create
>> > an "Example:" box, or the short-form "::" code-block syntax which will
>> > not apply QMP highlighting when used outside of the qmp-example
>> > directive.
>> >
>> > Why?
>> > ----
>> >
>> > This patch has several benefits:
>> >
>> > 1. Example sections can now be written more arbitrarily, mixing
>> >    explanatory paragraphs and code blocks however desired.
>> >
>> > 2. Example sections can now use fully arbitrary rST.
>> >
>> > 3. All code blocks are now lexed and validated as QMP; increasing
>> >    usability of the docs and ensuring validity of example snippets.
>> >
>> >    (To some extent - This patch only gaurantees it lexes correctly, not
>> >    that it's valid under the JSON or QMP grammars. It will catch most
>> >    small mistakes, however.)
>> >
>> > 4. Each qmp-example can be titled or annotated independently without
>> >    bypassing the QMP lexer/validator.
>> >
>> >    (i.e. code blocks are now for *code* only, so we don't have to
>> >    sacrifice exposition for having lexically valid examples.)
>> >
>> > NOTE: As with the "Notes" conversion patch,
>>
>> Commit d461c279737 (qapi: convert "Note" sections to plain rST).
>>
>
> Didn't have a stable commit ID at the time, will work it in if/when the
> notes patches hit main.

They have.

>> >                                             this patch (and those
>> >       preceding) may change the rendering order for Examples in the
>>
>> The three preceding ones, to be precise.
>>
>> >       current generator. The forthcoming qapidoc rewrite will fix this
>> >       by always generating documentation in source order.
>>
>> Conversions from "Example" section to plain reST may change order.  This
>> patch converts a test, and the preceding three convert the real uses.
>>
>> Does any of the patches actually change order?
>
> I do not actually know ...! It has the *potential* in the same exact way
> that the notes patch did, but I don't actually know if it *did*. My hunch
> is "no" because there's only one intermediate section we identified with
> the notes series, but I didn't exhaustively prove it. That's why I used the
> "may" weasel wording.

Alright, I checked.

In documentation of command announce-self, the example moves from after
the arguments to before.  Unwanted change.

I can keep it in place if I insert a TODO before the example like this:

    diff --git a/qapi/net.json b/qapi/net.json
    index 9a723e56b5..50bfd5b681 100644
    --- a/qapi/net.json
    +++ b/qapi/net.json
    @@ -930,6 +930,8 @@
     # switches.  This can be useful when network bonds fail-over the
     # active slave.
     #
    +# TODO: This line is a hack to separate the example from the body
    +#
     # .. qmp-example::
     #
     #     -> { "execute": "announce-self",

I had to delete the .doctrees cache to make sphinx-build generate
corrected output.

>> > Signed-off-by: John Snow <jsnow@redhat.com>
>> > ---
>> >  docs/devel/qapi-code-gen.rst    | 58 ++++++++++++++++++++++++++++-----
>> >  scripts/qapi/parser.py          | 10 +++++-
>> >  tests/qapi-schema/doc-good.json | 19 +++++++----
>> >  tests/qapi-schema/doc-good.out  | 26 ++++++++++-----
>> >  tests/qapi-schema/doc-good.txt  | 23 ++++++-------
>> >  5 files changed, 98 insertions(+), 38 deletions(-)
>> >
>> > diff --git a/docs/devel/qapi-code-gen.rst b/docs/devel/qapi-code-gen.rst
>> > index ae97b335cbf..2e10a3cbd69 100644
>> > --- a/docs/devel/qapi-code-gen.rst
>> > +++ b/docs/devel/qapi-code-gen.rst
>> > @@ -899,7 +899,7 @@ Documentation markup
>> >  ~~~~~~~~~~~~~~~~~~~~
>> >
>> >  Documentation comments can use most rST markup.  In particular,
>> > -a ``::`` literal block can be used for examples::
>> > +a ``::`` literal block can be used for pre-formatted text::
>> >
>> >      # ::
>> >      #
>> > @@ -995,8 +995,8 @@ line "Features:", like this::
>> >    # @feature: Description text
>> >
>> >  A tagged section begins with a paragraph that starts with one of the
>> > -following words: "Since:", "Example:"/"Examples:", "Returns:",
>> > -"Errors:", "TODO:".  It ends with the start of a new section.
>> > +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::
>> > @@ -1020,13 +1020,53 @@ detailing a relevant error condition. For
>> example::
>> >  A "Since: x.y.z" tagged section lists the release that introduced the
>> >  definition.
>> >
>> > -An "Example" or "Examples" section is rendered entirely
>> > -as literal fixed-width text.  "TODO" sections are not rendered at all
>> > -(they are for developers, not users of QMP).  In other sections, the
>> > -text is formatted, and rST markup can be used.
>> > +"TODO" sections are not rendered at all (they are for developers, not
>>
>> Drop "at all"?
>>
>
> Sure.
>
>
>>
>> > +users of QMP).  In other sections, the text is formatted, and rST markup
>> > +can be used.
>> > +
>> > +QMP Examples can be added by using the ``.. qmp-example::``
>> > +directive. In its simplest form, this can be used to contain a single
>> > +QMP code block which accepts standard JSON syntax with additional server
>> > +directionality indicators (``->`` and ``<-``), and elisions (``...``).
>> > +
>> > +Optionally, a plaintext title may be provided by using the ``:title:``
>> > +directive option. If the title is omitted, the example title will
>> > +default to "Example:".
>> > +
>> > +A simple QMP example::
>> > +
>> > +  # .. qmp-example::
>> > +  #    :title: Using query-block
>> > +  #
>> > +  #    -> { "execute": "query-block" }
>> > +  #    <- { ... }
>> > +
>> > +More complex or multi-step examples where exposition is needed before or
>> > +between QMP code blocks can be created by using the ``:annotated:``
>> > +directive option. When using this option, nested QMP code blocks must be
>> > +entered explicitly with rST's ``::`` syntax.
>>
>
> Telling on myself: you can use .. code-block:: QMP too, but I figured
> recommending "::" was shorter and sweeter. There are lots of minutiae here
> for people who haven't spent a long time reading and writing rST, so I
> tried to keep it short.

Makes sense.

>> > +
>> > +Highlighting in non-QMP languages can be accomplished by using the
>> > +``.. code-block:: lang`` directive, and non-highlighted text can be
>> > +achieved by omitting the language argument.
>> >
>> >  For example::
>> >
>> > +  # .. qmp-example::
>> > +  #    :annotated:
>> > +  #    :title: A more complex demonstration
>> > +  #
>> > +  #    This is a more complex example that can use
>> > +  #    ``arbitrary rST syntax`` in its exposition::
>> > +  #
>> > +  #      -> { "execute": "query-block" }
>> > +  #      <- { ... }
>> > +  #
>> > +  #    Above, lengthy output has been omitted for brevity.
>> > +
>> > +
>> > +Examples of complete definition documentation::
>> > +
>> >   ##
>> >   # @BlockStats:
>> >   #
>> > @@ -1058,11 +1098,11 @@ For example::
>> >   #
>> >   # Since: 0.14
>> >   #
>> > - # Example:
>> > + # .. qmp-example::
>> >   #
>> >   #     -> { "execute": "query-blockstats" }
>> >   #     <- {
>> > - #          ... lots of output ...
>> > + #          ...
>> >   #        }
>> >   ##
>> >   { 'command': 'query-blockstats',
>> > diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py
>> > index 6ad5663e545..adc85b5b394 100644
>> > --- a/scripts/qapi/parser.py
>> > +++ b/scripts/qapi/parser.py
>> > @@ -553,7 +553,7 @@ def get_doc(self) -> 'QAPIDoc':
>> >                      # Note: "sections" with two colons are left alone as
>> >                      # rST markup and not interpreted as a section heading.
>> >
>> > -                    # TODO: Remove this error sometime in 2025 or so
>> > +                    # TODO: Remove these errors sometime in 2025 or so
>> >                      # after we've fully transitioned to the new qapidoc
>> >                      # generator.
>> >
>> > @@ -567,6 +567,14 @@ def get_doc(self) -> 'QAPIDoc':
>> >                          )
>> >                          raise QAPIParseError(self, emsg)
>> >
>> > +                    if 'Example' in match.group(1):
>> > +                        emsg = (
>> > +                            f"The '{match.group(1)}' section is no longer 
>> > "
>> > +                            "supported. Please use the '.. qmp-example::' 
>> > "
>> > +                            "directive, or other suitable markup instead."
>> > +                        )
>> > +                        raise QAPIParseError(self, emsg)
>> > +
>> >                      doc.new_tagged_section(self.info, match.group(1))
>> >                      text = line[match.end():]
>> >                      if text:
>> > diff --git a/tests/qapi-schema/doc-good.json 
>> > b/tests/qapi-schema/doc-good.json
>> > index 107123f8a8d..c71d65cd51f 100644
>> > --- a/tests/qapi-schema/doc-good.json
>> > +++ b/tests/qapi-schema/doc-good.json
>> > @@ -172,12 +172,17 @@
>> >  #
>> >  #  Duis aute irure dolor
>> >  #
>> > -# Example:
>> > +# .. qmp-example::
>> > +#    :title: Ideal fast-food burger situation
>> >  #
>> > -#  -> in
>> > -#  <- out
>> > +#    -> "in"
>> > +#    <- "out"
>>
>> Heh, trickery to make the text right of -> and <- JSON.
>>
>
> O:-)
>
> It's maybe *slightly* bad form, but it does help illustrate how the new
> directive is transformed when using text output modes in the test suite, so
> I kept it. The new directive will simply not allow malformed JSON, so this
> seemed like the simplest way to cheese that.

I figure an alternative would be a more flexible "...".

Two hunks up, you dumb one down:

    - # Example:
    + # .. qmp-example::
      #
      #     -> { "execute": "query-blockstats" }
      #     <- {
    - #          ... lots of output ...
    + #          ...
      #        }

If qmp_lexer.py understood this more verbose ellipsis, we could use
something like

      #    -> ... input ...
      #    <- ... output ...

Worth the bother?  Certainly not just for tests.  Maybe for nicer
examples in real documentation?  Up to you!

>> >  #
>> > -# Examples:
>> > +# Examples::
>> > +#
>> > +#  - Not a QMP code block
>> > +#  - Merely a preformatted code block literal
>> > +#  It isn't even an rST list.
>> >  #  - *verbatim*
>> >  #  - {braces}
>>
>
> (And here, we test the use of non-QMP code block literals, esp. after the
> qmp-example directive, proving that language settings have been restored to
> defaults.)

Appreciated.

>> >  #
>> > @@ -199,11 +204,11 @@
>> >  # @cmd-feat1: a feature
>> >  # @cmd-feat2: another feature
>> >  #
>> > -# Example:
>> > +# .. qmp-example::
>> >  #
>> > -#  -> in
>> > +#    -> "this example"
>> >  #
>> > -#  <- out
>> > +#    <- "has no title"
>>
>> Same trickery.
>>
>
> Do you want that changed ... ?

It's just a test, good enough.

>> >  ##
>> >  { 'command': 'cmd-boxed', 'boxed': true,
>> >    'data': 'Object',
>> > diff --git a/tests/qapi-schema/doc-good.out 
>> > b/tests/qapi-schema/doc-good.out
>> > index bd876b6542d..eee18cd436a 100644
>> > --- a/tests/qapi-schema/doc-good.out
>> > +++ b/tests/qapi-schema/doc-good.out
>> > @@ -184,13 +184,21 @@ frobnicate
>> >   - Ut enim ad minim veniam
>> >
>> >   Duis aute irure dolor
>> > -    section=Example
>> > - -> in
>> > - <- out
>> > -    section=Examples
>> > +
>> > +.. qmp-example::
>> > +   :title: Ideal fast-food burger situation
>> > +
>> > +   -> "in"
>> > +   <- "out"
>> > +
>> > +Examples::
>> > +
>> > + - Not a QMP code block
>> > + - Merely a preformatted code block literal
>> > + It isn't even an rST list.
>> >   - *verbatim*
>> >   - {braces}
>> > -    section=None
>> > +
>> >  Note::
>> >     Ceci n'est pas une note
>> >      section=Since
>> > @@ -202,10 +210,12 @@ If you're bored enough to read this, go see a video 
>> > of boxed cats
>> >  a feature
>> >      feature=cmd-feat2
>> >  another feature
>> > -    section=Example
>> > - -> in
>> > +    section=None
>> > +.. qmp-example::
>> >
>> > - <- out
>> > +   -> "this example"
>> > +
>> > +   <- "has no title"
>> >  doc symbol=EVT_BOXED
>> >      body=
>> >
>> > diff --git a/tests/qapi-schema/doc-good.txt 
>> > b/tests/qapi-schema/doc-good.txt
>> > index 30d457e5488..cb37db606a6 100644
>> > --- a/tests/qapi-schema/doc-good.txt
>> > +++ b/tests/qapi-schema/doc-good.txt
>> > @@ -217,17 +217,16 @@ Notes:
>> >
>> >  Duis aute irure dolor
>> >
>> > +Example: Ideal fast-food burger situation:
>>
>
> No comment on the american making fast food burger jokes? :-(

I chuckled, but no witticism came to me, so...
>
>> >
>> > -Example
>> > -~~~~~~~
>> > +   -> "in"
>> > +   <- "out"
>> >
>> > -   -> in
>> > -   <- out
>> > -
>> > -
>> > -Examples
>> > -~~~~~~~~
>> > +Examples:
>> >
>> > +   - Not a QMP code block
>> > +   - Merely a preformatted code block literal
>> > +   It isn't even an rST list.
>> >     - *verbatim*
>> >     - {braces}
>> >
>> > @@ -261,13 +260,11 @@ Features
>> >  "cmd-feat2"
>> >     another feature
>> >
>> > +Example::
>> >
>> > -Example
>> > -~~~~~~~
>> > +   -> "this example"
>> >
>> > -   -> in
>> > -
>> > -   <- out
>> > +   <- "has no title"
>> >
>> >
>> >  "EVT_BOXED" (Event)
>>
>
> Does this adequately resolve your qualms about .txt rendering for examples?

I think you mean my "Rendering to text now loses the "Example" heading"
on "[PATCH 13/13] qapi: convert "Example" sections to rST".  The diff
above demonstrates it's no longer lost.

There's a tiny issue you may or may not want to address: an example
isn't always separated by a blank line.  To reproduce, add my TODO hack
to announce-self, and examine qemu-qmp-ref.txt:

    "announce-self" (Command)
    -------------------------

    Trigger generation of broadcast RARP frames to update network
    switches.  This can be useful when network bonds fail-over the active
    slave.


    Arguments
    ~~~~~~~~~

--> The members of "AnnounceParameters"
--> Example::

       -> { "execute": "announce-self",
            "arguments": {
                "initial": 50, "max": 550, "rounds": 10, "step": 50,
                "interfaces": ["vn2", "vn3"], "id": "bob" } }
       <- { "return": {} }