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

[John Snow <jsnow@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 5557 bytes --]

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:
>
>     -<section id="qapidoc-31">
>     -<h3><a class="toc-backref" href="#id13" role="doc-backlink"><code
> class="docutils literal notranslate"><span
> class="pre">guest-shutdown</span></code> (Command)</a><a class="headerlink"
> href="#qapidoc-31" title="Permalink to this heading"></a></h3>
>     +<section id="qapidoc-30">
>     +<h3><a class="toc-backref" href="#id13" role="doc-backlink"><code
> class="docutils literal notranslate"><span
> class="pre">guest-shutdown</span></code> (Command)</a><a class="headerlink"
> href="#qapidoc-30" title="Permalink to this heading"></a></h3>
>      <p>Initiate guest-activated shutdown.  Note: this is an asynchronous
>      shutdown request, with no guarantee of successful shutdown.</p>
>      <section id="qapidoc-28">
>     @@ -502,22 +502,20 @@ shutdown request, with no guarantee of s
>      </dd>
>      </dl>
>      </section>
>     -<section id="qapidoc-29">
>      <p>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
>      command to confirm the VM status is “shutdown”.</p>
>     -</section>
>     -<section id="qapidoc-30">
>     -<h4>Since<a class="headerlink" href="#qapidoc-30" title="Permalink to
> this heading"></a></h4>
>     +<section id="qapidoc-29">
>     +<h4>Since<a class="headerlink" href="#qapidoc-29" title="Permalink to
> this heading"></a></h4>
>      <p>0.15.0</p>
>      </section>
>      </section>
>
> The id changes muddy the waters.  With them manually removed:
>
>      <section id="qapidoc-31">
>      <h3><a class="toc-backref" href="#id13" role="doc-backlink"><code
> class="docutils literal notranslate"><span
> class="pre">guest-shutdown</span></code> (Command)</a><a class="headerlink"
> href="#qapidoc-31" title="Permalink to this heading"></a></h3>
>      <p>Initiate guest-activated shutdown.  Note: this is an asynchronous
>      shutdown request, with no guarantee of successful shutdown.</p>
>      <section id="qapidoc-28">
>     @@ -502,22 +502,20 @@ shutdown request, with no guarantee of s
>      </dd>
>      </dl>
>      </section>
>     -<section id="qapidoc-29">
>      <p>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
>      command to confirm the VM status is “shutdown”.</p>
>     -</section>
>      <section id="qapidoc-30">
>      <h4>Since<a class="headerlink" href="#qapidoc-30" title="Permalink to
> this heading"></a></h4>
>      <p>0.15.0</p>
>      </section>
>      </section>
>
> 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...

>

[-- Attachment #2: Type: text/html, Size: 7687 bytes --]