Re: [PATCH v3 04/47] qapi: modify docstrings to be sphinx-compatible

[John Snow <jsnow@redhat.com>]

On 9/28/20 11:39 PM, Cleber Rosa wrote:
> On Thu, Sep 24, 2020 at 08:28:17PM -0400, John Snow wrote:
>> I did not say "sphinx beautiful", just "sphinx compatible". They will
>> not throw errors when parsed and interpreted as ReST.
>>
>> Signed-off-by: John Snow <jsnow@redhat.com>
>> ---
>>   scripts/qapi/doc.py    | 10 +++++-----
>>   scripts/qapi/gen.py    |  6 ++++--
>>   scripts/qapi/parser.py |  9 +++++----
>>   3 files changed, 14 insertions(+), 11 deletions(-)
>>
>> diff --git a/scripts/qapi/doc.py b/scripts/qapi/doc.py
>> index 92f584edcf..c41e9d29f5 100644
>> --- a/scripts/qapi/doc.py
>> +++ b/scripts/qapi/doc.py
>> @@ -65,11 +65,11 @@ def texi_format(doc):
>>       Format documentation
>>   
>>       Lines starting with:
>> -    - |: generates an @example
>> -    - =: generates @section
>> -    - ==: generates @subsection
>> -    - 1. or 1): generates an @enumerate @item
>> -    - */-: generates an @itemize list
>> +    - ``|:`` generates an @example
>> +    - ``=:`` generates @section
>> +    - ``==:`` generates @subsection
>> +    - ``1.`` or ``1):`` generates an @enumerate @item
>> +    - ``*/-:`` generates an @itemize list
>>       """
>>       ret = ''
>>       doc = subst_braces(doc)
>> diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py
>> index bf5552a4e7..3d25a8cff4 100644
>> --- a/scripts/qapi/gen.py
>> +++ b/scripts/qapi/gen.py
>> @@ -154,9 +154,11 @@ def _bottom(self):
>>   
>>   @contextmanager
>>   def ifcontext(ifcond, *args):
>> -    """A 'with' statement context manager to wrap with start_if()/end_if()
>> +    """
>> +    A 'with' statement context manager to wrap with start_if()/end_if()
>>   
> 
> If you're making it compatible, why not take the time to give
> backticks to start_if and end_if?
> 

I forget. Must not have been a good reason, then...?

> Bonus points for setting the "meth" role, but not lost points for not
> doing it, as I understand this is beyond what you're attempting at
> this time.
> 

I remain unsold on using explicit roles for references in docstrings, 
because I'd like to keep the amount of Sphinx-ese syntax down to a 
minimum if I can. The double backticks are bad enough ...

>> -    *args: any number of QAPIGenCCode
>> +    :param ifcond: List of conditionals
>> +    :param args: any number of QAPIGenCCode
>>   
> 
> I would argue that this is not a strict sphinx compatibility change,
> but a fix to a broken docstring.
> 

Not entirely correct: the syntax of *args breaks document generation.

(OK, I could make QAPIGenCCode a reference ...)

> - Cleber.
>