Markus Armbruster <arm...@redhat.com> writes:
> John Snow <js...@redhat.com> writes:
>
>> Hi! This series is based on armbru/pull-qapi-2025-02-26.
>>
>> This series is a "minimum viable" version of the new QAPI documentation
>> system. It does the bare minimum under the new framework, saving nice
>> features for later.
>
> Not saved for later: a massive improvement of the generated
> documentation's looks and usability. Moreover, I hope the new generator
> will be easier to maintain than the old one, because its inner workings
> are closer to how Sphinx expects such things to work. Fair?
>
>>
>> Patches 3-29 implement the qapi_domain extension.
>> Patches 30-57 implement the qapidoc "Transmogrifier".
>>
>> This series is still "RFC" quality, though it's quite nearly actually
>> ready for inclusion. The "add transmogrifier test document" patch is not
>> intended for actual merge, it's just there to demonstrate the new
>> document generator by producing output in docs/manual/qapi/index.html.
>>
>> Known shortcomings in this series:
>>
>> - Still no new QAPI unit tests. I'll add those for next go-around.
>
> Not a blocker as far as I'm concerned, because I feel you're unlikely to
> run away from this :)
>
>> - No new documentation. Also for next revision. I'll document the QAPI
>> domain syntax and give a brief overview of how the transmogrifier
>> functions, and a quick rundown of any new rST syntax that may be
>> pertinent to QAPI documentation writers.
>
> Likewise.
>
>> - IFCOND information is still rendered in two places, we'll need to
>> decide where and how we want to render it.
>
> I'll have a look, and then we'll talk.
Two shortcomings, actually:
- IFCOND in definitions (enum, struct, union, alternate, command,
event) are rendered in two places.
Example: query-tpm has 'if': 'CONFIG_TPM'. The rendered
documentation looks like this:
Command query-tpm ["#if" "CONFIG_TPM"] (Since: 1.5)
*Availability*: "CONFIG_TPM"
Return information about the TPM device
Example::
[...]
With the old generator, it looks like
"query-tpm" (Command)
---------------------
Return information about the TPM device
Since
~~~~~
1.5
Example::
[...]
If
~~
"CONFIG_TPM"
So, three ways to present the information, none of them immediately
obvious to a casual reader. The easiest to guess right is perhaps
the "Availability" box.
The two new ways are in more conspicious spots than the old one is.
Not sure I like that; I believe ifconds are fairly uninteresting to
users of QMP most of the time. More on that below.
The "Availability" box is even more conspicious than the [#if ...]
bracket. It also uses more space.
For complex conditions, the [#if ...] bracket can make the first
line less readable. Example:
Command query-cpu-definitions ["#if" "TARGET_PPC or TARGET_ARM or
TARGET_I386 or TARGET_S390X or TARGET_MIPS or TARGET_LOONGARCH64 or
TARGET_RISCV"] (Since: 1.2)
*Availability*: "TARGET_PPC or TARGET_ARM or TARGET_I386 or
TARGET_S390X or TARGET_MIPS or TARGET_LOONGARCH64 or TARGET_RISCV"
Return a list of supported virtual CPU definitions
The first line matters. This tips the balance for me. Let's go
with the "Availability" box at least for now.
There's a symbol before "Availability" in the generated HTML. Its
meaning is less than obvious. I remember you explained it to me,
but I forgot. We can polish this later.
- IFCOND elsewhere is not rendered at all. This is a regression. The
old generator shows it like this:
"mptcp": "boolean" (optional) (**If: **"HAVE_IPPROTO_MPTCP")
enable multi-path TCP. (Since 6.1)
I believe the regression is tolerable for now, because the
information lost is only modestly useful for users of QMP, and the
massive improvements the new generator provides outweigh this small
loss.
Why only modestly useful? It tells the reader the thing's
availability depends on how QEMU was built. It also provides a clue
that should let a developer or sufficiently clever non-developer
rebuild QEMU to provide the thing. This would involve some code
spelunking.
>> - No QAPI namespace support ... yet. So we can't enable it for QMP, QGA
>> and QSD simultaneously just yet. I don't think it will be difficult.
[...]
