Re: [PATCH v2 07/21] docs/qapidoc: fix nested parsing under untagged sections

[Markus Armbruster]

John Snow <jsnow@redhat.com> writes:

> On Fri, Jun 28, 2024, 3:55 AM Markus Armbruster <armbru@redhat.com> wrote:
>
>> John Snow <jsnow@redhat.com> writes:
>>
>> > Sphinx does not like sections without titles, because it wants to
>> > convert every section into a reference. When there is no title, it
>> > struggles to do this and transforms the tree inproperly.
>> >
>> > Depending on the rST used, this may result in an assertion error deep in
>> > the docutils HTMLWriter.
>> >
>> > (Observed when using ".. admonition:: Notes" under such a section - When
>> > this is transformed with its own <title> element, Sphinx is fooled into
>> > believing this title belongs to the section and incorrect mutates the
>> > docutils tree, leading to errors during rendering time.)
>> >
>> > When parsing an untagged section (free paragraphs), skip making a hollow
>> > section and instead append the parse results to the prior section.
>> >
>> > Many Bothans died to bring us this information.
>> >
>> > Signed-off-by: John Snow <jsnow@redhat.com>
>> > Acked-by: Markus Armbruster <armbru@redhat.com>
>>
>> Generated HTML changes, but the diff is hard to review due to id
>> attribute changes all over the place.
>>
>> Generated qemu-ga-ref.7 also changes:
>>
>>     diff -rup old/qemu-ga-ref.7 new/qemu-ga-ref.7
>>     --- old/qemu-ga-ref.7       2024-06-27 10:42:21.466096276 +0200
>>     +++ new/qemu-ga-ref.7       2024-06-27 10:45:36.502414099 +0200
>>     @@ -397,6 +397,7 @@ shutdown request, with no guarantee of s
>>      .B \fBmode\fP: \fBstring\fP (optional)
>>      \(dqhalt\(dq, \(dqpowerdown\(dq (default), or \(dqreboot\(dq
>>      .UNINDENT
>>     +.sp
>>      This command does NOT return a response on success.  Success
>>      condition is indicated by the VM exiting with a zero exit status or,
>>      when running with \-\-no\-shutdown, by issuing the query\-status QMP
>>     @@ -1348,6 +1349,7 @@ the new password entry string, base64 en
>>      .B \fBcrypted\fP: \fBboolean\fP
>>      true if password is already crypt()d, false if raw
>>      .UNINDENT
>>     +.sp
>>      If the \fBcrypted\fP flag is true, it is the caller\(aqs
>> responsibility to
>>      ensure the correct crypt() encryption scheme is used.  This command
>>      does not attempt to interpret or report on the encryption scheme.
>>
>> We add vertical space.  Visible when viewed with man.  Looks like an
>> improvement to me.
>>
>> Here's the first of these two spots in HTML:

[...]

>> The id changes muddy the waters.  With them manually removed:

[...]

>> Makes no visual difference in my browser.
>>
>> Do these differences match your expectations?
>>
>
> Yep!
>
> It does change the output just a little, but Sphinx really doesn't like
> title-less sections.
>
> I thought the change looked fine, and I'm still planning on removing this
> old generator anyway, so...

I'll add to the commit message

    The resulting output changes are basically invisible.

Thanks!