On Wed, Jun 19, 2024, 8:49 AM Markus Armbruster <arm...@redhat.com> wrote:
> John Snow <js...@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 <js...@redhat.com> > > Acked-by: Stefan Hajnoczi <stefa...@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 >
