Re: [PATCH 19/29] qapi/qapi-schema.json: Put headers in their own doc-comment blocks

[Markus Armbruster]

Peter Maydell <address@hidden> writes:

> On Fri, 7 Feb 2020 at 15:35, Markus Armbruster <address@hidden> wrote:
>>
>> Peter Maydell <address@hidden> writes:
>>
>> > Our current QAPI doc-comment markup allows section headers
>> > (introduced with a leading '=' or '==') anywhere in any documentation
>> > comment.  This works for texinfo because the texi generator simply
>> > prints a texinfo heading directive at that point in the output
>> > stream.  For rST generation, since we're assembling a tree of
>> > docutils nodes, this is awkward because a new section implies
>> > starting a new section node at the top level of the tree and
>> > generating text into there.
>> >
>> > New section headings in the middle of the documentation of a command
>> > or event would be pretty nonsensical, and in fact we only ever output
>> > new headings using 'freeform' doc comment blocks whose only content
>> > is the single line of the heading, with two exceptions, which are in
>> > the introductory freeform-doc-block at the top of
>> > qapi/qapi-schema.json.
>> >
>> > Split that doc-comment up so that the heading lines are in their own
>> > doc-comment.  This will allow us to tighten the specification to
>> > insist that heading lines are always standalone, rather than
>> > requiring the rST document generator to look at every line in a doc
>> > comment block and handle headings in odd places.
>
>> I figure this is a minimally invasive patch to avoid complications in
>> your rST generator.  I'm afraid it sweeps the actual problem under the
>> rug, namely flaws in our parsing and representation of doc comments.
>>
>> The doc comment parser doesn't recognize headings.  Instead, that's done
>> somewhere in the bowels of the Texinfo generator.  Works as long as the
>> input is "sane", happily generates invalid Texinfo otherwise, see
>> tests/qapi-schema/doc-bad-section.json.
>>
>> The proper fix is to make the parser recognize headers in the places
>> where headers make sense, and reject them elsewhere.
>>
>> But maybe we don't have to.  Do you plan to support full rST in doc
>> comments?  If yes, why have our own syntax for headings?  Why not leave
>> it to rST?  If no, do you plan to support a subset of rST?  If yes,
>> define it, please.  How will it be enforced?
>
> Doc comments do support full rST. However, (as the commit message
> here notes), if you're generating a tree of docutils nodes and
> one of them has a section heading in it then you'll get a result
> that looks like this:
>
> [root]
>   - [ some section created by the script for a QAPI command ]
>   - [ some section ]
>       - [text nodes, etc going into this section]
>       - [a section resulting from rST parsing the header inside the docstring]
>   - [ next section created by the script for a QAPI command ]
>
> (ie you'll have defined a subsection within whatever document
> paragraph/section the current command is documenting, not
> a new top-level subsection which subsequent commands will
> become children of)
>
> What you actually want is that the new header results in
> a differently structured tree:
> [root]
>   - [ some section created by the script for a QAPI command ]
>   - [ some section ]
>       - [text nodes, etc going into this section]
>   - [ a new top level section whose header is whatever this header is ]
>      - [ next section created by the script is a child of that section ]
>      - [ etc ]
>
> There's no way to get that without actually noticing and handling
> headings specially as being something entirely different from
> a lump of documentation text. "A heading is a single-line special-case
> of a freeform comment" happens to be the way we mark up headings
> now in 99% of cases, so that's what I implemented. (The Sphinx
> extension will complain if there's trailing junk lines after
> a heading line at the beginning of a freeform comment block.
> If you use '== something' in a line in the middle of a doc
> comment, we'll just interpret that as rST source, which is to
> say a couple of literal equals signs at the start of a line.)

A couple of remarks.

Silently passing a "# == something" line to rST for literal
(mis-)interpretation is not nice.  It's the kind of indifference that
led to the messes you cleaned up in PATCH 04 and 08.  If the '=' markup
is only valid in certain places, it should be rejected where it isn't.

By refusing to translate "# == something" to rST (silently or loudly,
doesn't matter), the first tree structure becomes impossible.  Except
when I do the translating myself: I can put an *rST* section wherever I
want.

I'm still having difficulties understanding what exactly we gain by
translating '=' markup to rST.

By the way, your implementation rejects

    ##
    # = Introduction
    # xxx
    ##

but silently accepts

    ##
    # xxx
    # = Introduction
    ##

doc-good.json has more instances of this issue.  Before your series, we
actually check we generate the Texinfo we expect for it.  I can't find
where you cover this now.  It has saved me from my screwups more than
once, so I don't want to lose that.

Now let's put my doubts and your possible bugs / omissions aside and
assume we want '=' markup, and we want to keep the resulting sections
out of "sections created by the script for a QAPI command".

A schema's documentation is a sequence of comment blocks.

Each comment block is either a definition comment block or a free form
comment block.

Before your series, we recognize '=' markup everywhere, but that's
basically wrong (see "flaws in our parsing and representation of doc
comments" above).  It should be accepted only in free-form comment
blocks.

That way, the free-form comment blocks build a section structure, and
the definition comment blocks slot their stuff into this structure.

Form a language design perspective, I can't see the need for restricting
'=' further to occur only by themselves.

Is it an issue of implementation perhaps?