Re: docs/qemu-qmp-ref.7 markup messed up

[Markus Armbruster <armbru@redhat.com>]

John Snow <jsnow@redhat.com> writes:

> On 3/26/21 10:19 AM, Markus Armbruster wrote:
>> When I look at docs/qemu-qmp-ref.7 with less -R, I see
>>     ReplicationMode (Enum)
>>         An enumeration of replication modes.
>>     Values
>>         primary
>>                Primary mode, the vm's state will be sent to secondary QEMU.
>>         secondary
>>                Secondary mode, receive the vm's state from primary QEMU.
>>     Since
>>         2.9
>>     If
>> -->    defined(CONFIG_REPLICATION).SS BlockdevOptionsReplication (Object)
>>         Driver specific block device options for replication
>>     Members
>>         mode: ReplicationMode
>>                the replication mode
>> The line I marked with --> is bad.  It should instead look like
>>     If
>>         defined(CONFIG_REPLICATION)
>>     BlockdevOptionsReplication (Object)
>>         Driver specific block device options for replication
>> Unformatted code:
>>      .SS \fBReplicationMode\fP (Enum)
>>      .sp
>>      An enumeration of replication modes.
>>      .SS Values
>>      .INDENT 0.0
>>      .TP
>>      .B \fBprimary\fP
>>      Primary mode, the vm\(aqs state will be sent to secondary QEMU.
>>      .TP
>>      .B \fBsecondary\fP
>>      Secondary mode, receive the vm\(aqs state from primary QEMU.
>>      .UNINDENT
>>      .SS Since
>>      .sp
>>      2.9
>>      .SS If
>> --> \fBdefined(CONFIG_REPLICATION)\fP.SS \fBBlockdevOptionsReplication\fP (Object)
>>      .sp
>>      Driver specific block device options for replication
>>      .SS Members
>>      .INDENT 0.0
>>      .TP
>>      .B \fBmode\fP: \fBReplicationMode\fP
>>      the replication mode
>>      .TP
>> I believe line I marked with --> should be broken before .SS.
>> I'm using sphinx-build-3 2.2.2.
>> I checked with the merge commit that switched QAPI doc generation to
>> Sphinx (commit e344ffe73b), same result.
>> 
>
> It looks like that's consistent for every case I can see for
> "defined(...)", where the .SS bit comes immediately on the same line.
>
> _nodes_for_if_section seems to handle the docutil tree creation for
> the stuff in question, here, I think?
>
> I changed the heading to "IfZ" to make it nicer to grep, and then If I
> pepper in some prints (mercifully docutils has very nice __str__ 
> methods!) I can see this type of stuff:
>
> <section
> ids="qapidoc-713"><title>IfZ</title><literal>defined(CONFIG_REPLICATION)</literal></section>
> <section
> ids="qapidoc-717"><title>IfZ</title><literal>defined(CONFIG_REPLICATION)</literal></section>
>
> Hm, I think <literal> is not a block-level element, and maybe there's
> a bug/intentional-design-choice/bug where it can't handle a non-block 
> child of a section correctly?
>
> Let me try wrapping it in a PARAGRAPH node...
>
> .SS IfZ
> .sp
> \fBdefined(CONFIG_REPLICATION)\fP
> .SS \fBBlockdevOptionsReplication\fP (Object)
> .sp
>
>
> That looks better, I think.
>
>
> diff --git a/docs/sphinx/qapidoc.py b/docs/sphinx/qapidoc.py
> index b7b86b5dff..b7a2d39c10 100644
> --- a/docs/sphinx/qapidoc.py
> +++ b/docs/sphinx/qapidoc.py
> @@ -278,7 +278,9 @@ def _nodes_for_if_section(self, ifcond):
>          nodelist = []
>          if ifcond:
>              snode = self._make_section('If')
> -            snode += self._nodes_for_ifcond(ifcond, with_if=False)
> +            snode += nodes.paragraph(
> +                '', '', *self._nodes_for_ifcond(ifcond, with_if=False)
> +            )
>              nodelist.append(snode)
>          return nodelist
>
>
> Signed-off-by: John Snow <jsnow@redhat.com>
>
> --js

Fixes the issue for me, thanks!

Tested-by: Markus Armbruster <armbru@redhat.com>

Please post as a proper patch.