Re: [PATCH 2/8] qapi: prohibit 'details' sections between tagged sections

[Markus Armbruster <armbru@redhat.com>]

John Snow <jsnow@redhat.com> writes:

> On Fri, Mar 20, 2026, 8:46 AM Markus Armbruster <armbru@redhat.com> wrote:
>
>> John Snow <jsnow@redhat.com> writes:
>>
>> > This patch prohibits plain documentation sections from appearing between
>> > "tagged" sections. The two existing uses of this pattern are patched
>> > out.
>>
>> One real use, and one just for test coverage.
>>
>> > This is being done primarily to ensure consistency between the source
>> > documents and the final, rendered HTML output. Because
>> > member/feature/returns/error sections will always appear in a visually
>> > grouped element in the HTML output, prohibiting plain paragraphs between
>> > those sections ensures ordering consistency between source and the final
>> > render.
>>
>> Is consistency an actual problem being fixed, or a future problem being
>> avoided?  I'm guessing the latter, based on my review of qom.json below.
>>
>
> With just one actual use, it's *mostly* avoiding future problems. It does
> correct one instance in the rendered HTML of a plain text paragraph
> interrupting the tabular fields, which is not a "bug" per se but a visual
> inconsistency I wish to eliminate.

A visual blemish you wish to eliminate is not an inconsistency between
doc source and rendered doc.

It would become one if you made the generator move the PLAIN section out
to fix the blemish, but that's not the plan.

Instead, you fix the blemish by making doc writers move their PLAIN
sections out.

> The problem is minimal now, but intensifies when considering inlining.

Would inlining make it necessary for the generator to move PLAIN
sections around?  Why?

If yes, this patch eliminates a visual blemish now *and* avoids
inconsistency later.

> Prohibiting the insertion of any section into the "tabular region" helps
> ensure consistent, good looking HTML manual output.
>
> A goal is for source order to match rendered order. Rendered order looks
> best with all tabular elements grouped together without root-level
> paragraphs interrupting them.
>
> Thus, the desire to restrict source order.
>
> (i.e. we allow only one intro section and only one details section.)

I'm not opposed to that, I just want a clearer commit message :)

>> > Additionally, prohibiting such "middle" text paragraphs allows us to
>> > classify all plain text sections as either "intro" or "details" sections,
>> > because these sections must either appear before structured/tagged
>> > sections ("intro") or afterwards ("details").
>>
>> Huh?
>>
>> The previous patch already classified all plain text sections as either
>> INTRO or DETAILS.
>>
>> I think the paragraph would make sense if this patch came before the
>> previous one.
>>
>
> Mmm.
>
> The previous patch is confusingly worded and I didn't do better here.
>
> Previous patch categorizes all plaintext sections as intro or details, but
> technically allows multiple details sections.
>
> This patch enforces that we only have one.
>
> (...I think. Currently not clear on how TODO and Since behave with this
> logic. Maybe we end up with multiple details sections interrupted only by
> non-rendered sections... Edit: yes, you find such a case below.)
>
>
>> > This keeps the inlining algorithm simpler with fewer "splice" points
>> > when merging multiple documentation blocks.
>> >
>> > Signed-off-by: John Snow <jsnow@redhat.com>
>> > ---
>> >  qapi/qom.json                   |  4 ++--
>> >  scripts/qapi/parser.py          | 17 +++++++++++++++++
>> >  tests/qapi-schema/doc-good.json |  4 ++--
>> >  tests/qapi-schema/doc-good.out  |  4 ++--
>> >  tests/qapi-schema/doc-good.txt  |  8 ++++----
>> >  5 files changed, 27 insertions(+), 10 deletions(-)
>> >
>> > diff --git a/qapi/qom.json b/qapi/qom.json
>> > index c653248f85d..1b47abd44e9 100644
>> > --- a/qapi/qom.json
>> > +++ b/qapi/qom.json
>> > @@ -243,12 +243,12 @@
>> >  #
>> >  # @typename: the type name of an object
>> >  #
>> > +# Returns: a list describing object properties
>> > +#
>> >  # .. note:: Objects can create properties at runtime, for example to
>> >  #    describe links between different devices and/or objects.  These
>> >  #    properties are not included in the output of this command.
>> >  #
>> > -# Returns: a list describing object properties
>> > -#
>> >  # Since: 2.12
>> >  ##
>> >  { 'command': 'qom-list-properties',
>>
>> Rendered documentation before the patch:
>>
>>     Command qom-list-properties (Since: 2.12)
>>
>>        List properties associated with a QOM object.
>>
>>        Arguments:
>>           * typename (string) -- the type name of an object
>>
>>        Note:
>>
>>          Objects can create properties at runtime, for example to describe
>>          links between different devices and/or objects.  These properties
>>          are not included in the output of this command.
>>
>>        Return:
>>           [ObjectPropertyInfo] -- a list describing object properties
>>
>> Afterwards:
>>
>>     Command qom-list-properties (Since: 2.12)
>>
>>        List properties associated with a QOM object.
>>
>>        Arguments:
>>           * typename (string) -- the type name of an object
>>
>>        Return:
>>           [ObjectPropertyInfo] -- a list describing object properties
>>
>>        Note:
>>
>>          Objects can create properties at runtime, for example to describe
>>          links between different devices and/or objects.  These properties
>>          are not included in the output of this command.
>>
>> Fine.
>>
>> [Skipping the Python code in my first pass...]
>>
>> > diff --git a/tests/qapi-schema/doc-good.json b/tests/qapi-schema/doc-good.json
>> > index fac13425b72..9103fed472e 100644
>> > --- a/tests/qapi-schema/doc-good.json
>> > +++ b/tests/qapi-schema/doc-good.json
>> > @@ -165,12 +165,12 @@
>> >  # @cmd-feat1: a feature
>> >  # @cmd-feat2: another feature
>> >  #
>> > -# .. note:: @arg3 is undocumented
>> > -#
>> >  # Returns: @Object
>> >  #
>> >  # Errors: some
>> >  #
>> > +# .. note:: @arg3 is undocumented
>> > +#
>> >  # TODO: frobnicate
>> >  #
>> >  # .. admonition:: Notes
>> > diff --git a/tests/qapi-schema/doc-good.out b/tests/qapi-schema/doc-good.out
>> > index 04e29e8d50f..6a0167ad580 100644
>> > --- a/tests/qapi-schema/doc-good.out
>> > +++ b/tests/qapi-schema/doc-good.out
>> > @@ -175,12 +175,12 @@ description starts on the same line
>> >  a feature
>> >      feature=cmd-feat2
>> >  another feature
>> > -    section=Details
>> > -.. note:: @arg3 is undocumented
>> >      section=Returns
>> >  @Object
>> >      section=Errors
>> >  some
>> > +    section=Details
>> > +.. note:: @arg3 is undocumented
>> >      section=Todo
>> >  frobnicate
>> >      section=Details
>>
>> The Details section is still between tagged sections.  The code
>> prohibiting it must have a hole :)
>>
>
> Well, there's the answer to my above self-musing on Since/TODO...
> After reading my replies, is it clear what I am concerned with
> accomplishing?
>
> If not, the goal for *rendered output* is this:
>
> 1. Intro (literally and truly only ever one section, both in the QAPIDoc
> sense and in the rendered HTML output sense)
>
> 2. All tabular sections (members, returns, Errors, features)
>
> 3. Details
>
> You'll notice I didn't bother specifying Since and TODO here. TODO is
> removed from rendered output, so I don't actually care where it goes in the
> QAPIDoc source list.
>
> Since is also removed from the flow in the HTML output, so I also don't
> care about it. (However, you requested that it specifically go last, so I
> do address that in this series and later prohibit any details sections from
> appearing after a Since marker.)

Understood.

> The true "semantic" goals here are two things:
>
> (1) Prohibiting free text from appearing within the tabular section region
>
> (2) Delineating free text that appears before the tabular region from free
> text that appears after it.
>
> I apologize for conflating "section order" with "rendered section order".
>
> You are right to point out that these sections i do not care about may, in
> the source, impact the classification of other sections and the total
> number thereof.
>
> (i.e. TODO is enough to terminate the intro without an explicit details
> marker, and Since/TODO can create multiple details sections after the
> tabular region. They are merged visually but not in the QAPIDoc sections
> list. I don't consider this a bug per se, but it is the source of a lot of
> confusion here in this series when I am eliding that technicality.)

A commit message needs to explain why and how the patch is useful.  When
details distract from the core argument too much, it can be beneficial
to gloss over them.

Glossing over is not the same as not mentioning.

Commit message readers use it to build a mental model of things.  A good
model simplifies reality.  A bad model misleads about reality.

If the commit message completely elides details that matter, readers end
up with bad models and unwarranted confidence in them.  They'll then
struggle with the patch.

If the commit message at least hints at the details elided, readers may
still end up with bad models, but they'll hopefully understand their
limitations, treat them as incomplete, and fill in the details from the
patch.

[...]