Re: [PATCH v5 16/36] qapi/common.py: Convert comments into docstrings, and elaborate

[Markus Armbruster]

John Snow <jsnow@redhat.com> writes:

> On 10/7/20 5:14 AM, Markus Armbruster wrote:
>> John Snow <jsnow@redhat.com> writes:
>> 
>>> As docstrings, they'll show up in documentation and IDE help.
>>>
>>> The docstring style being targeted is the Sphinx documentation
>>> style. Sphinx uses an extension of ReST with "domains". We use the
>>> (implicit) Python domain, which supports a number of custom "info
>>> fields". Those info fields are documented here:
>>> https://www.sphinx-doc.org/en/master/usage/restructuredtext/domains.html#info-field-lists
>>>
>>> Primarily, we use `:param X: descr`, `:return[s]: descr`, and `:raise[s]
>>> Z: when`. Everything else is the Sphinx dialect of ReST.
>>>
>>> (No, nothing checks or enforces this style that I am aware of. Sphinx
>>> either chokes or succeeds, but does not enforce a standard of what is
>>> otherwise inside the docstring. Pycharm does highlight when your param
>>> fields are not aligned with the actual fields present. It does not
>>> highlight missing return or exception statements. There is no existing
>>> style guide I am aware of that covers a standard for a minimally
>>> acceptable docstring. I am debating writing one.)
>>>
>>> Signed-off-by: John Snow <jsnow@redhat.com>
>>> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
>>> Reviewed-by: Cleber Rosa <crosa@redhat.com>
>>> ---
>>>   scripts/qapi/common.py | 53 +++++++++++++++++++++++++++++++-----------
>>>   1 file changed, 39 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
>>> index 74a2c001ed9..0ef38ea5fe0 100644
>>> --- a/scripts/qapi/common.py
>>> +++ b/scripts/qapi/common.py
>>> @@ -15,15 +15,24 @@
>>>   from typing import Optional, Sequence
>>>     
>>> +#: Sentinel value that causes all space to its right to be removed.
>> What's the purpose of : after # ?
>> 
>
> Documents this name in Sphinx. We had a small discussion about it, I
> think; "Does using this special form or the docstring make the comment 
> visible in any IDE?" (No.)
>
> There's no Python-AST way to document these, but there is a Sphinx way
> to document them, so I did that.
>
> (Doing it like this allows `EATSPACE` to be used as a cross-reference.)

Thanks.

Consider pointing this out when you write the comment & doc string part
of our Python style guide.

>> I'm not sure this is a "sentinel value".  Wikipedia:
>>      In computer programming, a sentinel value (also referred to as a
>>      flag value, trip value, rogue value, signal value, or dummy data)[1]
>>      is a special value in the context of an algorithm which uses its
>>      presence as a condition of termination, typically in a loop or
>>      recursive algorithm.
>>      https://en.wikipedia.org/wiki/Sentinel_value
>> 
>
> I really should try to learn English as a second language so I know
> what any of the words I use mean, I guess. I had slipped to a less
> strict usage where it meant more like "placeholder".
>
>> Perhaps
>>     # Magic string value that gets removed along with all space to
>> the
>>     # right.
>> 
>
> This can be written on one line if we gently disregard the 72 column
> limit. (Maybe you already did when you wrote it and my client wrapped 
> it. Who knows!)

Drop the period and it fits ;-P

You could also drop "value" without loss.

[...]