Re: [PATCH 20/20] qapi: convert "Example" sections to rST
John Snow writes:
> Eliminate the "Example" sections in QAPI doc blocks, converting them
> into QMP example code blocks. This is generally done by converting
> "Example:" or "Examples:" lines into ".. code-block:: QMP" lines.
>
> This patch does also allow for the use of the rST syntax "Example::" by
> exempting double-colon syntax from the QAPI doc parser, but that form is
> not used by this conversion patch. The phrase "Example" here is not
> special, it is the double-colon syntax that transforms the following
> block into a code-block. By default, *this* form does not apply QMP
> highlighting.
>
> This patch has several benefits:
>
> 1. Example sections can now be written more arbitrarily, mixing
>explanatory paragraphs and code blocks however desired.
>
> 2. Example sections can now use fully arbitrary rST.
>
> 3. All code blocks are now lexed and validated as QMP; increasing
>usability of the docs and ensuring validity of example snippets.
>
> 4. Each code-block can be captioned independently without bypassing the
>QMP lexer/validator.
>
> For any sections with more than one example, examples are split up into
> multiple code-block regions. If annotations are present, those
> annotations are converted into code-block captions instead, e.g.
>
> ```
> Examples:
>
>1. Lorem Ipsum
>
>-> { "foo": "bar" }
> ```
>
> Is rewritten as:
>
> ```
> .. code-block:: QMP
>:caption: Example: Lorem Ipsum
>
>-> { "foo": "bar" }
> ```
>
> This process was only semi-automated:
>
> 1. Replace "Examples?:" sections with sed:
>
> sed -i 's|# Example:|# .. code-block:: QMP|' *.json
> sed -i 's|# Examples:|# .. code-block:: QMP|' *.json
>
> 2. Identify sections that no longer parse successfully by attempting the
>doc build, convert annotations into captions manually.
>(Tedious, oh well.)
>
> 3. Add captions where still needed:
>
> sed -zi 's|# .. code-block:: QMP\n#\n|# .. code-block:: QMP\n#:caption:
> Example\n#\n|g' *.json
>
> Not fully ideal, but hopefully not something that has to be done very
> often. (Or ever again.)
>
> Signed-off-by: John Snow
> ---
> qapi/acpi.json | 6 +-
> qapi/block-core.json| 120 --
> qapi/block.json | 60 +++--
> qapi/char.json | 36 ++--
> qapi/control.json | 16 ++--
> qapi/dump.json | 12 ++-
> qapi/machine-target.json| 3 +-
> qapi/machine.json | 79 ++---
> qapi/migration.json | 145 +++-
> qapi/misc-target.json | 33 +---
> qapi/misc.json | 48 +++
> qapi/net.json | 30 +--
> qapi/pci.json | 6 +-
> qapi/qapi-schema.json | 6 +-
> qapi/qdev.json | 15 +++-
> qapi/qom.json | 20 +++--
> qapi/replay.json| 12 ++-
> qapi/rocker.json| 12 ++-
> qapi/run-state.json | 45 ++
> qapi/tpm.json | 9 +-
> qapi/trace.json | 6 +-
> qapi/transaction.json | 3 +-
> qapi/ui.json| 62 +-
> qapi/virtio.json| 38 +
> qapi/yank.json | 6 +-
> scripts/qapi/parser.py | 15 +++-
> tests/qapi-schema/doc-good.json | 12 +--
> tests/qapi-schema/doc-good.out | 17 ++--
> tests/qapi-schema/doc-good.txt | 17 +---
> 29 files changed, 574 insertions(+), 315 deletions(-)
Missing: update of docs/devel/qapi-code-gen.rst.
> diff --git a/qapi/acpi.json b/qapi/acpi.json
> index aa4dbe57943..3da01f1b7fc 100644
> --- a/qapi/acpi.json
> +++ b/qapi/acpi.json
> @@ -111,7 +111,8 @@
> #
> # Since: 2.1
> #
> -# Example:
> +# .. code-block:: QMP
> +#:caption: Example
I wish this was a bit less verbose. Oh well, we'll live.
> #
> # -> { "execute": "query-acpi-ospm-status" }
> # <- { "return": [ { "device": "d1", "slot": "0", "slot-type": "DIMM",
> "source": 1, "status": 0},
This is rendered as a light green box with the caption on top, in
italics and centered. I'm not sure I like the use of the caption. The
previous patch's Note boxes look nicer.
The contents of the box is highlighted. I am sure I like that.
> @@ -131,7 +132,8 @@
> #
> # Since: 2.1
> #
> -# Example:
> +# .. code-block:: QMP
> +#:caption: Example
> #
> # <- { "event": "ACPI_DEVICE_OST",
> # "data": { "info": { "device": "d1", "slot": "0",
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 530af40404d..bb0447207df 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -763,7 +763,8 @@
> #
> # Since: 0.14
> #
> -# Example:
> +# .. code-block:: QMP
> +#:caption: Example
> #
> # -> { "execute": "query-block" }
> # <- {
> @@ -1167,7 +1168,8 @@
> #
> # Since: 0.14
> #
> -# Example:
> +# .. code
Re: [PATCH 19/20] qapi: convert "Note" sections to plain rST
John Snow writes:
> We do not need a dedicated section for notes. By eliminating a specially
> parsed section, these notes can be treated as normal rST paragraphs in
> the new QMP reference manual, and can be placed and styled much more
> flexibly.
>
> Update the QAPI parser to now prohibit special "Note" sections while
> suggesting a new syntax.
>
> The exact formatting to use is a matter of taste, but a good candidate
> is simply:
>
> .. note:: lorem ipsum ...
>
> but there are other choices, too. The Sphinx readthedocs theme offers
> theming for the following forms (capitalization unimportant); all are
> adorned with a (!) symbol in the title bar for rendered HTML docs.
>
> These are rendered in orange:
>
> .. Attention:: ...
> .. Caution:: ...
> .. WARNING:: ...
>
> These are rendered in red:
>
> .. DANGER:: ...
> .. Error:: ...
>
> These are rendered in green:
>
> .. Hint:: ...
> .. Important:: ...
> .. Tip:: ...
>
> These are rendered in blue:
>
> .. Note:: ...
> .. admonition:: custom title
>
>admonition body text
>
> This patch uses ".. notes::" almost everywhere, with just two "caution"
> directives. ".. admonition:: notes" is used in a few places where we had
> an ordered list of multiple notes.
>
> Signed-off-by: John Snow
> ---
> qapi/block-core.json | 30 +++
> qapi/block.json | 2 +-
> qapi/char.json| 12 +--
> qapi/control.json | 15 ++--
> qapi/dump.json| 2 +-
> qapi/introspect.json | 6 +-
> qapi/machine-target.json | 26 +++---
> qapi/machine.json | 47 +-
> qapi/migration.json | 12 +--
> qapi/misc.json| 88 +--
> qapi/net.json | 6 +-
> qapi/pci.json | 7 +-
> qapi/qdev.json| 30 +++
> qapi/qom.json | 19 ++--
> qapi/rocker.json | 16 ++--
> qapi/run-state.json | 18 ++--
> qapi/sockets.json | 10 +--
> qapi/stats.json | 22 ++---
> qapi/transaction.json | 8 +-
> qapi/ui.json | 29 +++---
> qapi/virtio.json | 12 +--
> qga/qapi-schema.json | 48 +-
> scripts/qapi/parser.py| 9 ++
> tests/qapi-schema/doc-empty-section.err | 2 +-
> tests/qapi-schema/doc-empty-section.json | 2 +-
> tests/qapi-schema/doc-good.json | 6 +-
> tests/qapi-schema/doc-good.out| 10 ++-
> tests/qapi-schema/doc-good.txt| 14 ++-
> .../qapi-schema/doc-interleaved-section.json | 2 +-
> 29 files changed, 258 insertions(+), 252 deletions(-)
Missing: update of docs/devel/qapi-code-gen.rst. Something like
diff --git a/docs/devel/qapi-code-gen.rst b/docs/devel/qapi-code-gen.rst
index f453bd3546..1a4e240adb 100644
--- a/docs/devel/qapi-code-gen.rst
+++ b/docs/devel/qapi-code-gen.rst
@@ -995,14 +995,13 @@ line "Features:", like this::
# @feature: Description text
A tagged section begins with a paragraph that starts with one of the
-following words: "Note:"/"Notes:", "Since:", "Example:"/"Examples:",
-"Returns:", "Errors:", "TODO:". It ends with the start of a new
-section.
+following words: "Since:", "Example:"/"Examples:", "Returns:",
+"Errors:", "TODO:". It ends with the start of a new section.
The second and subsequent lines of tagged sections must be indented
like this::
- # Note: Ut enim ad minim veniam, quis nostrud exercitation ullamco
+ # TODO: Ut enim ad minim veniam, quis nostrud exercitation ullamco
# laboris nisi ut aliquip ex ea commodo consequat.
#
# Duis aute irure dolor in reprehenderit in voluptate velit esse
>
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 64fe5240cc9..530af40404d 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -1510,7 +1510,7 @@
> # @mode: whether and how QEMU should create a new image, default is
> # 'absolute-paths'.
> #
> -# Note: Either @device or @node-name must be set but not both.
> +# .. note:: Either @device or @node-name must be set but not both.
The commit message talks about ".. Note::", but you actually use
".. note::". Is there a difference?
> #
> ##
> { 'struct': 'BlockdevSnapshotSync',
> @@ -1616,9 +1616,9 @@
> #
> # @unstable: Member @x-perf is experimental.
> #
> -# Note: @on-source-error and @on-target-error only affect background
> -# I/O. If an error occurs during a guest write request, the
> -# device's rerror/werror actions will be used.
> +# .. note:: @on-source-error and @on-target-error only affect backg
Re: [PATCH 18/20] qapi: ensure all errors sections are uniformly typset
John Snow writes: > Transactions have the only instance of an Errors section that isn't a > rST list; turn it into one. Just for consistency? Or do you have other shenanigans up your sleeve? If we want the Errors sections to remain all rST lists, we should update docs/devel/qapi-code-gen.rst to say so. > Signed-off-by: John Snow
Re: [PATCH 14/20] qapi: fix non-compliant JSON examples
John Snow writes: > If we parse all examples as QMP, we need them to conform to a standard > so that they render correctly. Once the QMP lexer is active for > examples, these will produce warning messages and fail the build. > > The QMP lexer still supports elisions, but they must be represented as > the value "...", so two examples have been adjusted to support that > format here. I think this could use a bit more context. I believe you're referring to docs/sphinx/qmp_lexer.py. It describes itself as "a Sphinx extension that provides a QMP lexer for code blocks." "If we parse all examples as QMP" and "Once the QMP lexer is active for examples" suggests we're *not* using it for (some?) examples. So what are we using it for? > Signed-off-by: John Snow Patch looks lovely. Hat tip to Victor Toso, who fixed up most examples two years ago. Back then we couldn't decide how to do elisions, so we left some unfixed.
Re: [PATCH 13/20] docs/qapidoc: fix nested parsing under untagged sections
John Snow 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.
I'm getting vibes of someone having had hours of "fun" with Sphinx...
Can you give you an idea of how a reproducer would look like?
> 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.
Terribly sad.
> Signed-off-by: John Snow
> ---
> docs/sphinx/qapidoc.py | 16 +++-
> 1 file changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/docs/sphinx/qapidoc.py b/docs/sphinx/qapidoc.py
> index 34e95bd168d..cfc0cf169ef 100644
> --- a/docs/sphinx/qapidoc.py
> +++ b/docs/sphinx/qapidoc.py
> @@ -286,14 +286,20 @@ def _nodes_for_sections(self, doc):
> if section.tag and section.tag == 'TODO':
> # Hide TODO: sections
> continue
> +
> +if not section.tag:
> +# Sphinx cannot handle sectionless titles;
> +# Instead, just append the results to the prior section.
> +container = nodes.container()
> +self._parse_text_into_node(section.text, container)
> +nodelist += container.children
> +continue
> +
> snode = self._make_section(section.tag)
> -if section.tag and section.tag.startswith('Example'):
> +if section.tag.startswith('Example'):
> snode += self._nodes_for_example(dedent(section.text))
> else:
> -self._parse_text_into_node(
> -dedent(section.text) if section.tag else section.text,
> -snode,
> -)
> +self._parse_text_into_node(dedent(section.text), snode)
> nodelist.append(snode)
> return nodelist
Looks plausible. I lack the Sphinx-fu to say more.
Re: [PATCH 11/20] qapi/schema: add doc_visible property to QAPISchemaDefinition
John Snow writes: > The intent here is to mark only certain definitions as visible in the > end-user docs. > > All commands and events are inherently visible. Everything else is > visible only if it is a member (or a branch member) of a type that is > visible, or if it is named as a return type for a command. > > Notably, this excludes arg_type for commands and events, and any > base_types specified for structures/unions. Those objects may still be > marked visible if they are named as members from a visible type. Why? I figure the answer is "because the transmogrifier inlines the things excluded". Correct? > This does not necessarily match the data revealed by introspection: in > this case, we want anything that we are cross-referencing in generated > documentation to be available to target. I don't get the part after the colon. > Some internal and built-in types may be marked visible with this > approach, but if they do not have a documentation block, they'll be > skipped by the generator anyway. This includes array types and built-in > primitives which do not get their own documentation objects. > > This information is not yet used by qapidoc, which continues to render > documentation exactly as it has. This information will be used by the > new qapidoc (the "transmogrifier"), to be introduced later. The new > generator verifies that all of the objects that should be rendered *are* > by failing if any cross-references are missing, verifying everything is > in place. So... we decide "doc should be visible" here, and then the transmogrifier decides again, and we check the two decisions match? > Signed-off-by: John Snow
Re: [PATCH 09/20] qapi/parser: add undocumented stub members to all_sections
John Snow writes:
> This helps simplify the doc generator if it doesn't have to check for
> undocumented members.
>
> Signed-off-by: John Snow
> ---
> scripts/qapi/parser.py | 20 ++--
> 1 file changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py
> index b1794f71e12..3cd8e7ee295 100644
> --- a/scripts/qapi/parser.py
> +++ b/scripts/qapi/parser.py
> @@ -740,8 +740,24 @@ def connect_member(self, member: 'QAPISchemaMember') ->
> None:
> raise QAPISemError(member.info,
> "%s '%s' lacks documentation"
> % (member.role, member.name))
> -self.args[member.name] = QAPIDoc.ArgSection(
> -self.info, '@' + member.name, 'member')
> +
> +# Insert stub documentation section for missing member docs.
> +section = QAPIDoc.ArgSection(
> +self.info, f"@{member.name}", "member")
Although I like f-strings in general, I'd pefer to stick to '@' +
member.name here, because it's simpler.
Also, let's not change 'member' to "member". Existing practice: single
quotes for string literals unless double quotes avoid escapes. Except
English prose (like error messages) is always in double quotes.
> +self.args[member.name] = section
> +
> +# Determine where to insert stub doc.
If we have some member documentation, the member doc stubs clearly must
go there. Inserting them at the end makes sense.
Else we want to put them where the parser would accept real member
documentation.
"The parser" is .get_doc(). This is what it accepts (I'm prepared to
explain this in detail if necessary):
One untagged section
Member documentation, if any
Zero ore more tagged or untagged sections
Feature documentation, if any
Zero or more tagged or untagged sections
If we there is no member documentation, this is
One untagged section
Zero ore more tagged or untagged sections
Feature documentation, if any
Zero or more tagged or untagged sections
Note that we cannot have two adjacent untagged sections (we only create
one if the current section isn't untagged; if it is, we extend it
instead). Thus, the second section must be tagged or feature
documentation.
Therefore, the member doc stubs must go right after the first section.
This is also where qapidoc.py inserts member documentation.
> +index = 0
> +for i, sect in enumerate(self.all_sections):
> +# insert after these:
> +if sect.kind in ('intro-paragraph', 'member'):
> +index = i + 1
> +# but before these:
> +elif sect.kind in ('tagged', 'feature', 'outro-paragraph'):
> +index = i
> +break
Can you describe what this does in English? As a specification; simply
paraphrasing the code is cheating. I tried, and gave up.
Above, I derived what I believe we need to do. It's simple enough: if
we have member documentation, it starts right after the first (untagged)
section, and the stub goes to the end of the member documentation.
Else, the stub goes right after the first section.
Code:
index = 1;
while self.all_sections[index].kind == 'member':
index += 1
Of course future patches I haven't seen might change the invariants in
ways that break my simple code. We'll see.
> +self.all_sections.insert(index, section)
> +
> self.args[member.name].connect(member)
>
> def connect_feature(self, feature: 'QAPISchemaFeature') -> None:
