John Snow <jsnow@redhat.com> 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.
>
> Convert all existing "Note" and "Notes" sections to pure rST. As part of
> the conversion, capitalize the first letter of each sentence and add
> trailing punctuation where appropriate to ensure notes look sensible and
> consistent in rendered HTML documentation.
>
> Update docs/devel/qapi-code-gen.rst to reflect the new paradigm, and ...
>
> ... Update the QAPI parser to prohibit "Note" sections while suggesting
Scratch "... ..." and downcase "Update"?
> 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.
>
> See
> https://sphinx-rtd-theme.readthedocs.io/en/stable/demo/demo.html#admonitions
> for examples of each directive/admonition in use.
>
> 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 ".. note::" almost everywhere, with just two "caution"
> directives. ".. admonition:: notes" is used in a few places where we had
> an ordered list of multiple notes that would not make sense as
> standalone/separate admonitions.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> Acked-by: Stefan Hajnoczi <stefanha@redhat.com> [for block*.json]
[...]
> diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
> index b3de1fb6b3a..57598331c5c 100644
> --- a/qga/qapi-schema.json
> +++ b/qga/qapi-schema.json
> @@ -422,8 +422,9 @@
> # Returns: GuestFsfreezeStatus ("thawed", "frozen", etc., as defined
> # below)
> #
> -# Note: This may fail to properly report the current state as a result
> -# of some other guest processes having issued an fs freeze/thaw.
> +# .. note:: This may fail to properly report the current state as a
> +# result of some other guest processes having issued an fs
> +# freeze/thaw.
> #
> # Since: 0.15.0
> ##
> @@ -443,9 +444,9 @@
> #
> # Returns: Number of file systems currently frozen.
> #
> -# Note: On Windows, the command is implemented with the help of a
> -# Volume Shadow-copy Service DLL helper. The frozen state is
> -# limited for up to 10 seconds by VSS.
> +# .. note:: On Windows, the command is implemented with the help of a
> +# Volume Shadow-copy Service DLL helper. The frozen state is limited
> +# for up to 10 seconds by VSS.
> #
> # Since: 0.15.0
> ##
> @@ -479,10 +480,10 @@
> #
> # Returns: Number of file systems thawed by this call
> #
> -# Note: if return value does not match the previous call to
> -# guest-fsfreeze-freeze, this likely means some freezable
> -# filesystems were unfrozen before this call, and that the
> -# filesystem state may have changed before issuing this command.
> +# .. note:: If return value does not match the previous call to
> +# guest-fsfreeze-freeze, this likely means some freezable filesystems
> +# were unfrozen before this call, and that the filesystem state may
> +# have changed before issuing this command.
> #
> # Since: 0.15.0
> ##
> @@ -560,8 +561,8 @@
> # Errors:
> # - If suspend to disk is not supported, Unsupported
> #
> -# Notes: It's strongly recommended to issue the guest-sync command
> -# before sending commands when the guest resumes
> +# .. note:: It's strongly recommended to issue the guest-sync command
> +# before sending commands when the guest resumes.
> #
> # Since: 1.1
> ##
> @@ -596,8 +597,8 @@
> # Errors:
> # - If suspend to ram is not supported, Unsupported
> #
> -# Notes: It's strongly recommended to issue the guest-sync command
> -# before sending commands when the guest resumes
> +# .. note:: It's strongly recommended to issue the guest-sync command
> +# before sending commands when the guest resumes.
> #
> # Since: 1.1
> ##
> @@ -631,8 +632,8 @@
> # Errors:
> # - If hybrid suspend is not supported, Unsupported
> #
> -# Notes: It's strongly recommended to issue the guest-sync command
> -# before sending commands when the guest resumes
> +# .. note:: It's strongly recommended to issue the guest-sync command
> +# before sending commands when the guest resumes.
> #
> # Since: 1.1
> ##
> @@ -1461,16 +1462,15 @@
> # * POSIX: as defined by os-release(5)
> # * Windows: contains string "server" or "client"
> #
> -# Notes: On POSIX systems the fields @id, @name, @pretty-name,
> -# @version, @version-id, @variant and @variant-id follow the
> -# definition specified in os-release(5). Refer to the manual page
> -# for exact description of the fields. Their values are taken
> -# from the os-release file. If the file is not present in the
> -# system, or the values are not present in the file, the fields
> -# are not included.
> +# .. note:: On POSIX systems the fields @id, @name, @pretty-name,
> +# @version, @version-id, @variant and @variant-id follow the
> +# definition specified in os-release(5). Refer to the manual page for
> +# exact description of the fields. Their values are taken from the
> +# os-release file. If the file is not present in the system, or the
> +# values are not present in the file, the fields are not included.
> #
> -# On Windows the values are filled from information gathered from
> -# the system.
> +# On Windows the values are filled from information gathered from
> +# the system.
Please don't change the indentation here. I get the same output with
@@ -1461,7 +1462,7 @@
# * POSIX: as defined by os-release(5)
# * Windows: contains string "server" or "client"
#
-# Notes: On POSIX systems the fields @id, @name, @pretty-name,
+# .. note:: On POSIX systems the fields @id, @name, @pretty-name,
# @version, @version-id, @variant and @variant-id follow the
# definition specified in os-release(5). Refer to the manual page
# for exact description of the fields. Their values are taken
> #
> # Since: 2.10
> ##
> diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py
> index dfd6a6c5bee..53b06a94508 100644
> --- a/scripts/qapi/parser.py
> +++ b/scripts/qapi/parser.py
> @@ -548,6 +548,21 @@ def get_doc(self) -> 'QAPIDoc':
> r'(Returns|Errors|Since|Notes?|Examples?|TODO): *',
> line):
> # tagged section
> +
> + # TODO: Remove this error sometime in 2025 or so
> + # after we've fully transitioned to the new qapidoc
> + # generator.
> +
> + # See commit message for more markup suggestions O:-)
> + if 'Note' in match.group(1):
> + emsg = (
> + f"The '{match.group(1)}' section is no longer "
> + "supported. Please use rST's '.. note::' or "
> + "'.. admonition:: notes' directives, or another "
> + "suitable admonition instead."
> + )
> + raise QAPIParseError(self, emsg)
> +
> doc.new_tagged_section(self.info, match.group(1))
> text = line[match.end():]
> if text:
> diff --git a/tests/qapi-schema/doc-empty-section.err b/tests/qapi-schema/doc-empty-section.err
> index 5f03a6d733f..711a0d629c2 100644
> --- a/tests/qapi-schema/doc-empty-section.err
> +++ b/tests/qapi-schema/doc-empty-section.err
> @@ -1 +1 @@
> -doc-empty-section.json:6: text required after 'Note:'
> +doc-empty-section.json:6: text required after 'Errors:'
> diff --git a/tests/qapi-schema/doc-empty-section.json b/tests/qapi-schema/doc-empty-section.json
> index f3384e9a3bb..f179d3eff6d 100644
> --- a/tests/qapi-schema/doc-empty-section.json
> +++ b/tests/qapi-schema/doc-empty-section.json
> @@ -3,6 +3,6 @@
> ##
> # @foo:
> #
> -# Note:
> +# Errors:
> ##
> { 'command': 'foo', 'data': {'a': 'int'} }
> diff --git a/tests/qapi-schema/doc-good.json b/tests/qapi-schema/doc-good.json
> index 8b39eb946af..4b338cc0186 100644
> --- a/tests/qapi-schema/doc-good.json
> +++ b/tests/qapi-schema/doc-good.json
> @@ -29,7 +29,7 @@
> # - Second list
> # Note: still in list
> #
> -# Note: not in list
> +# .. note:: not in list
Uh... see [*] below.
> #
> # 1. Third list
> # is numbered
> @@ -157,7 +157,7 @@
> # @cmd-feat1: a feature
> # @cmd-feat2: another feature
> #
> -# Note: @arg3 is undocumented
> +# .. note:: @arg3 is undocumented
> #
> # Returns: @Object
> #
> @@ -165,7 +165,7 @@
> #
> # TODO: frobnicate
> #
> -# Notes:
> +# .. admonition:: Notes
> #
> # - Lorem ipsum dolor sit amet
> # - Ut enim ad minim veniam
> diff --git a/tests/qapi-schema/doc-good.out b/tests/qapi-schema/doc-good.out
> index 435f6e6d768..2c9b4e419cb 100644
> --- a/tests/qapi-schema/doc-good.out
> +++ b/tests/qapi-schema/doc-good.out
> @@ -76,7 +76,7 @@ Not in list
> - Second list
> Note: still in list
>
> -Note: not in list
[*] This demonstrates the "Note: ..." is *not* recognized as a "Note"
section before the patch (compare to the change we get for recognized
sections below).
Obscure fact: the doc parser recognizes tagged sections *only* in
definition documentation. Arguably a misfeature.
Your series makes the misfeature even more obscure, because afterwards,
the only tagged section left that could make sense in a free-form doc
comment is "TODO". Let's not worry about the misfeature now.
Right, it's gonna go away or be heavily reduced. A fish for another fry.
Two sensible solutions:
1. Since the patch converts tagged sections, and this isn't, don't touch
it. If you feel you want to mention this in the commit message, go
ahead.
Oh, uh. Alright. I wonder why I changed it then. I thought I was playing error message whackamole with this one, but maybe I did do a regexp at some point.
I'll leave it be if I can. If I can't, for some reason, then ...
2. Touch it anyway. Do mention it in the commit message then.
... This, with why I couldn't leave it be.
> +.. note:: not in list
>
> 1. Third list
> is numbered
> @@ -169,15 +169,17 @@ description starts on the same line
> a feature
> feature=cmd-feat2
> another feature
> - section=Note
> -@arg3 is undocumented
> + section=None
> +.. note:: @arg3 is undocumented
> section=Returns
> @Object
> section=Errors
> some
> section=TODO
> frobnicate
> - section=Notes
> + section=None
> +.. admonition:: Notes
> +
> - Lorem ipsum dolor sit amet
> - Ut enim ad minim veniam
>
> diff --git a/tests/qapi-schema/doc-good.txt b/tests/qapi-schema/doc-good.txt
> index 847db70412d..b89f35d5476 100644
> --- a/tests/qapi-schema/doc-good.txt
> +++ b/tests/qapi-schema/doc-good.txt
> @@ -17,7 +17,9 @@ Not in list
>
> * Second list Note: still in list
>
> -Note: not in list
> +Note:
> +
> + not in list
>
> 1. Third list is numbered
>
> @@ -193,11 +195,9 @@ Features
> "cmd-feat2"
> another feature
>
> +Note:
>
> -Note
> -~~~~
> -
> -"arg3" is undocumented
> + "arg3" is undocumented
>
>
> Returns
> @@ -211,9 +211,7 @@ Errors
>
> some
>
> -
> -Notes
> -~~~~~
> +Notes:
>
> * Lorem ipsum dolor sit amet
>
> diff --git a/tests/qapi-schema/doc-interleaved-section.json b/tests/qapi-schema/doc-interleaved-section.json
> index adb29e98daa..b26bc0bbb79 100644
> --- a/tests/qapi-schema/doc-interleaved-section.json
> +++ b/tests/qapi-schema/doc-interleaved-section.json
> @@ -10,7 +10,7 @@
> #
> # bao
> #
> -# Note: a section.
> +# Returns: a section.
> #
> # @foobar: catch this
> #
"Returns:" is only valid for commands, and this is a struct. Let's use
"TODO:" instead.
Weird that it didn't prohibit it. Bug?
--js