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

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

[Markus Armbruster]

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.

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

[John Snow]

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

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

[Markus Armbruster]

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.