John Snow <js...@redhat.com> writes:
> On Fri, Jun 14, 2024 at 10:39 AM Markus Armbruster <arm...@redhat.com>
> wrote:
>
>> John Snow <js...@redhat.com> writes:
>>
>> > Eliminate the "Example" sections in QAPI doc blocks, converting them
>> > into QMP example code blocks. This is generally done by converting
>> > "Example:" or "Examples:" lines into ".. code-block:: QMP" lines.
>> >
>> > This patch does also allow for the use of the rST syntax "Example::" by
>> > exempting double-colon syntax from the QAPI doc parser, but that form is
>> > not used by this conversion patch. The phrase "Example" here is not
>> > special, it is the double-colon syntax that transforms the following
>> > block into a code-block. By default, *this* form does not apply QMP
>> > highlighting.
>> >
>> > 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.
>> >
>> > 4. Each code-block can be captioned independently without bypassing the
>> > QMP lexer/validator.
>> >
>> > For any sections with more than one example, examples are split up into
>> > multiple code-block regions. If annotations are present, those
>> > annotations are converted into code-block captions instead, e.g.
>> >
>> > ```
>> > Examples:
>> >
>> > 1. Lorem Ipsum
>> >
>> > -> { "foo": "bar" }
>> > ```
>> >
>> > Is rewritten as:
>> >
>> > ```
>> > .. code-block:: QMP
>> > :caption: Example: Lorem Ipsum
>> >
>> > -> { "foo": "bar" }
>> > ```
>> >
>> > This process was only semi-automated:
>> >
>> > 1. Replace "Examples?:" sections with sed:
>> >
>> > sed -i 's|# Example:|# .. code-block:: QMP|' *.json
>> > sed -i 's|# Examples:|# .. code-block:: QMP|' *.json
>> >
>> > 2. Identify sections that no longer parse successfully by attempting the
>> > doc build, convert annotations into captions manually.
>> > (Tedious, oh well.)
>> >
>> > 3. Add captions where still needed:
>> >
>> > sed -zi 's|# .. code-block:: QMP\n#\n|# .. code-block:: QMP\n# :caption:
>> > Example\n#\n|g' *.json
>> >
>> > Not fully ideal, but hopefully not something that has to be done very
>> > often. (Or ever again.)
>> >
>> > Signed-off-by: John Snow <js...@redhat.com>
>> > ---
>> > qapi/acpi.json | 6 +-
>> > qapi/block-core.json | 120 ++++++++++++++++----------
>> > qapi/block.json | 60 +++++++------
>> > qapi/char.json | 36 ++++++--
>> > qapi/control.json | 16 ++--
>> > qapi/dump.json | 12 ++-
>> > qapi/machine-target.json | 3 +-
>> > qapi/machine.json | 79 ++++++++++-------
>> > qapi/migration.json | 145 +++++++++++++++++++++++---------
>> > qapi/misc-target.json | 33 +++++---
>> > qapi/misc.json | 48 +++++++----
>> > qapi/net.json | 30 +++++--
>> > qapi/pci.json | 6 +-
>> > qapi/qapi-schema.json | 6 +-
>> > qapi/qdev.json | 15 +++-
>> > qapi/qom.json | 20 +++--
>> > qapi/replay.json | 12 ++-
>> > qapi/rocker.json | 12 ++-
>> > qapi/run-state.json | 45 ++++++----
>> > qapi/tpm.json | 9 +-
>> > qapi/trace.json | 6 +-
>> > qapi/transaction.json | 3 +-
>> > qapi/ui.json | 62 +++++++++-----
>> > qapi/virtio.json | 38 +++++----
>> > qapi/yank.json | 6 +-
>> > scripts/qapi/parser.py | 15 +++-
>> > tests/qapi-schema/doc-good.json | 12 +--
>> > tests/qapi-schema/doc-good.out | 17 ++--
>> > tests/qapi-schema/doc-good.txt | 17 +---
>> > 29 files changed, 574 insertions(+), 315 deletions(-)
>>
>> Missing: update of docs/devel/qapi-code-gen.rst.
>>
>> > diff --git a/qapi/acpi.json b/qapi/acpi.json
>> > index aa4dbe57943..3da01f1b7fc 100644
>> > --- a/qapi/acpi.json
>> > +++ b/qapi/acpi.json
>> > @@ -111,7 +111,8 @@
>> > #
>> > # Since: 2.1
>> > #
>> > -# Example:
>> > +# .. code-block:: QMP
>> > +# :caption: Example
>>
>> I wish this was a bit less verbose. Oh well, we'll live.
>>
>
> We can create a custom directive that assumes a default caption; e.g.
>
> .. qapi:example::
>
> ... blah blah blah ...
>
> And if you want to override it, you'd just
>
> .. qapi:example::
> :caption: Lorem Ipsum ...
>
> ... blah blah blah ...
>
> Interested? (I kept this patch vanilla to avoid getting fancy, but there
> are fanciness options available if you'd like them.)
Let's keep it simple for now.
>> > #
>> > # -> { "execute": "query-acpi-ospm-status" }
>> > # <- { "return": [ { "device": "d1", "slot": "0", "slot-type":
>> > "DIMM", "source": 1, "status": 0},
>>
>> This is rendered as a light green box with the caption on top, in
>> italics and centered. I'm not sure I like the use of the caption. The
>> previous patch's Note boxes look nicer.
>>
>
> We can change that with styling - my dedicated CSS intern was busy with
> finals when I wrote this patch ;)
Tell her I asked for another helping of her magic!
>> The contents of the box is highlighted. I am sure I like that.
>>
>
> Yes.
>
> [...]
>
>> > -# Example:
>> > -#
>> > -# Set new histograms for all io types with intervals
>> > -# [0, 10), [10, 50), [50, 100), [100, +inf):
>> > +# .. code-block:: QMP
>> > +# :caption: Example:
>> > +# Set new histograms for all io types with intervals
>> > +# [0, 10), [10, 50), [50, 100), [100, +inf):
>>
>> Captions long enough to be rendered as multiple lines look particularly
>> bad to me. The centering...
>>
>
> Will attempt to address it with CSS. I do agree, just wasn't time to hammer
> it out.
>
> [...]
>
>
>> > @@ -134,7 +136,8 @@
>> > #
>> > # Since: 0.14
>> > #
>> > -# Example:
>> > +# .. code-block:: QMP
>> > +# :caption: Example
>> > #
>> > # -> { "execute": "query-commands" }
>> > # <- {
>> > @@ -149,8 +152,8 @@
>> > # ]
>> > # }
>> > #
>> > -# Note: This example has been shortened as the real response is too
>> > -# long.
>> > +# This example has been shortened as the real response is too long.
>>
>> Squash into the previous patch?
>>
>
> OK
>
> [...]
>
>
>> > diff --git a/qapi/pci.json b/qapi/pci.json
>> > index f51159a2c4c..9192212661b 100644
>> > --- a/qapi/pci.json
>> > +++ b/qapi/pci.json
>> > @@ -182,7 +182,8 @@
>> > #
>> > # Since: 0.14
>> > #
>> > -# Example:
>> > +# .. code-block:: QMP
>> > +# :caption: Example
>> > #
>> > # -> { "execute": "query-pci" }
>> > # <- { "return": [
>> > @@ -311,8 +312,7 @@
>> > # ]
>> > # }
>> > #
>> > -# Note: This example has been shortened as the real response is too
>> > -# long.
>> > +# This example has been shortened as the real response is too long.
>>
>> Squash into the previous patch?
>>
>
> OK
>
>
>>
>> > #
>> > ##
>> > { 'command': 'query-pci', 'returns': ['PciInfo'] }
>> > diff --git a/qapi/qapi-schema.json b/qapi/qapi-schema.json
>> > index 5e33da7228f..66fbcbd3619 100644
>> > --- a/qapi/qapi-schema.json
>> > +++ b/qapi/qapi-schema.json
>> > @@ -20,11 +20,7 @@
>> > # understand. However, in real protocol usage, they're emitted as a
>> > # single line.
>> > #
>> > -# Also, the following notation is used to denote data flow:
>> > -#
>> > -# Example:
>> > -#
>> > -# ::
>> > +# Also, the following notation is used to denote data flow::
>> > #
>> > # -> data issued by the Client
>> > # <- Server data response
>>
>> No use of caption here. Looks better, I think.
>>
>
> OK - Let me play around with the styling, because I do want to have some
> kind of form option available for cargo-culting to add captions or an
> explanation of some kind. If I can't make it look good with CSS, I'll
> capitulate and mark them up as alternating normal paragraphs and examples.
>
> Forbidding "Examples?:" was just an easy way to make sure I converted
> everything; and especially to catch any late merges ... I am hesitant to go
> that route for maintainability. But, if you want to volunteer to play
> whack-a-mole for the next few releases, then...
Making use of the old tag a hard error is a smart move. But I'm
prepared to sacrifice it for more nicely formatted examples.
> (Also, this example doesn't use the QMP lexer, because it's not real QMP.
Yes.
> It could be cajoled by making the lines string values, for example - or
> making it a more representative example that actually resembles QMP.)
No need unless it actually improves the generated docs.
>> > diff --git a/qapi/qdev.json b/qapi/qdev.json
>> > index d031fc3590d..cfe403fea20 100644
>> > --- a/qapi/qdev.json
>> > +++ b/qapi/qdev.json
>> > @@ -62,7 +62,8 @@
>> > # the ``-device DEVICE,help`` command-line argument, where DEVICE
>> > # is the device's name.
>> > #
>> > -# Example:
>> > +# .. code-block:: QMP
>> > +# :caption: Example
>> > #
>> > # -> { "execute": "device_add",
>> > # "arguments": { "driver": "e1000", "id": "net1",
>>
>> How does
>>
>> # Example:
>> +# .. code-block:: QMP
>> #
>> # -> { "execute": "device_add",
>> # "arguments": { "driver": "e1000", "id": "net1",
>>
>> look? Requires nerfing the error you add to parser.py.
>>
>
> Undesirable, IMO -- but "Example::" alongside an option to choose the QMP
> lexer by default for QMP docs may be acceptable. I can demo some choices
> for you on a screenshare call if you'd like to workshop this aesthetic
> choice out together.
>
> [...]
>
>
>> > diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py
>> > index 8b1da96124e..afc0b444034 100644
>> > --- a/scripts/qapi/parser.py
>> > +++ b/scripts/qapi/parser.py
>> > @@ -554,9 +554,12 @@ def get_doc(self) -> 'QAPIDoc':
>> > no_more_args = True
>> > intro = False
>> > elif match := re.match(
>> > - r'(Returns|Errors|Since|Notes?|Examples?|TODO):
>> > *',
>> > +
>> > r'(Returns|Errors|Since|Notes?|Examples?(?!::)|TODO)'
>> > + r': *',
>> > line):
>>
>> Hmm, I wonder...
>>
>>
>> https://www.sphinx-doc.org/en/master/usage/restructuredtext/basics.html#literal-blocks
>> has:
>>
>> Literal code blocks (ref) are introduced by ending a paragraph with
>> the special marker ::.
>>
>> Not capturing regular rST markup like
>>
>> Example::
>>
>> mumble mumble
>>
>> for our own purposes makes sense. But it makes exactly as much sense
>> for any of the tags, doesn'it?
>>
>> Should we instead change the regexp to match only when there's a
>> *single* colon?
>>
>
> OK. My regexp-fu is maybe weak, but I think I can just put (?!::): at the
> end of this regex without tying it to Examples, and I'll move that into its
> own patch.
>
>>
>>
>> > - # tagged section
>> > + # tagged section.
>>
>> Spurious comment change.
>>
>
> A *beautiful* comment change. An *inspired* comment change.
>
> (OK, removing it...)
>
>
>>
>> > + # Examples sections followed by two colons are
>> > excluded;
>> > + # those are raw rST syntax!
>> >
>> > if 'Note' in match.group(1):
>> > emsg = (
>> > @@ -566,6 +569,14 @@ def get_doc(self) -> 'QAPIDoc':
>> > )
>> > raise QAPIParseError(self, emsg)
>> >
>> > + if match.group(1).startswith("Example"):
>> > + emsg = (
>> > + f"The '{match.group(1)}' section is
>> > deprecated. "
>> > + "Please use rST's '.. code-block:: QMP'
>> > directive,"
>> > + " 'Example::', or other suitable markup
>> > instead."
>> > + )
>> > + raise QAPIParseError(self, emsg)
>> > +
>>
>> I guess this will be helpful while people get used to the changed
>> syntax. Once they are, I'd like to get rid of it. Same for "Note"
>> right above.
>>
>
> Yeah - the thinking was that it would help buffer the transitional period
> and could be removed after a release or two. I'll update the phrasing to
> not use "deprecated", also.
Throw in a TODO comment to remind us.
>> > 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 0a294eb324e..57e2e591938 100644
>> > --- a/tests/qapi-schema/doc-good.json
>> > +++ b/tests/qapi-schema/doc-good.json
>> > @@ -46,11 +46,12 @@
>> > #
>> > # Duis aute irure dolor
>> > #
>> > -# Example:
>> > +# .. code-block:: QMP
>>
>> No captions here?
>>
>
> They aren't *required*, I just liked having a dedicated place to put 'em in
> the rendered output for our real docs.
If captions are optional, doc-good should have at least one with
caption, and one without caption.
> [...]
>
>
>> I want this just as much as the previous patch.
>>
>>
> okie-dokey, I'll include it in the mini-fork of the pre-req series.
