qemu-block
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 09/13] qapi: convert "Note" sections to plain rST


From: John Snow
Subject: Re: [PATCH 09/13] qapi: convert "Note" sections to plain rST
Date: Thu, 20 Jun 2024 14:44:58 -0400



On Thu, Jun 20, 2024 at 11:46 AM John Snow <jsnow@redhat.com> wrote:


On Thu, Jun 20, 2024, 9:35 AM Markus Armbruster <armbru@redhat.com> wrote:
Markus Armbruster <armbru@redhat.com> writes:

> John Snow <jsnow@redhat.com> writes:

[...]

>> 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

[...]

>> @@ -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

I'm blind.  Actually, you change indentation of subsequent lines from 4
to 3 *everywhere*.  I guess you do that to make subsequent lines line up
with the directive, here "note".

Everywhere else, we indent such lines by 4.  Hmm.  How terrible would it
be not to mess with the alignment?

If we want to use 3 for directives, is it worth pointing out in the
commit message?

[...]

Let me look up some conventions and see what's the most prominent... as well as testing what emacs does by default (or if emacs can be configured to do what we want with in-tree style config. Warning: I am functionally inept at emacs lisp. Warning 2x: [neo]vi[m] users, you're entirely on your own. I'm sorry.)

I use three myself by force of habit and have some mild reluctance to respin for that reason, but ... guess we ought to be consistent if we can.

(No idea where my habit came from. Maybe it is just because it looks nice to me and no other reason.)

((I have no plans, nor desire, to write any kind of checker to enforce this, though - sorry.))

Sphinx doc uses three spaces: https://www.sphinx-doc.org/en/master/usage/restructuredtext/basics.html#directives

... but it warns that it's arbitrary; but it seems common to align with the directive.

* https://docutils.sourceforge.io/docs/ref/rst/restructuredtext.html#footnotes
   footnotes require "at least 3" spaces

* https://docutils.sourceforge.io/docs/ref/rst/restructuredtext.html#directives
  directives are only required to be "indented" but the amount isn't specified. rst docs use three.

I'm happy with three; I don't believe we need to make it consistent with e.g. our home-spun field list syntax (arguments, features) or with code blocks. I think whatever looks good in the source is fine, and I think three looks good for directives. I don't think we need to require this, but I can mention in the commit message that I chose it for the sake of aesthetics and for parity with what most rST docs appear to use.

Note: emacs behavior for wrapping paragraphs in our QAPI files does not create an indent if there isn't already one. I think convincing emacs to apply rST rules inside of a JSON file we lie and call a Python file might be beyond my ability to do quickly.

The default behavior for my emacs (which I haven't customized very much, if at all) in our source tree for *.rst files is to wrap directive lines with a three space indent.

So, I'm happy saying this is a good way to do it.

--js

reply via email to

[Prev in Thread] Current Thread [Next in Thread]