qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: John Snow
Subject: Re: [PATCH 8/8] qapi: remove "Example" doc section
Date: Wed, 10 Jul 2024 10:46:40 -0400



On Wed, Jul 10, 2024 at 1:45 AM Markus Armbruster <armbru@redhat.com> wrote:
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!

Good idea. I'll have to see if it's easy to get the lexer to realize both "..." and "...(etc)..." as tokens without accidentally eating legitimate JSON between. Probably a standalone patch/miniseries.
 

>> >  #
>> > -# 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": {} }


Oh, uh. I'm not sure how to fix that right away, but I'll try. No promises.

reply via email to

[Prev in Thread] Current Thread [Next in Thread]