diff mbox series

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

Message ID 20200925002900.465855-5-jsnow@redhat.com
State New
Headers show
Series qapi: static typing conversion, pt1 | expand

Commit Message

John Snow Sept. 25, 2020, 12:28 a.m. UTC
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(-)

Comments

Cleber Rosa Sept. 29, 2020, 3:39 a.m. UTC | #1
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?

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.

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

- Cleber.
John Snow Sept. 29, 2020, 3:37 p.m. UTC | #2
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.
>
diff mbox series

Patch

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()
 
-    *args: any number of QAPIGenCCode
+    :param ifcond: List of conditionals
+    :param args: any number of QAPIGenCCode
 
     Example::
 
diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py
index 165925ca72..00738cea8c 100644
--- a/scripts/qapi/parser.py
+++ b/scripts/qapi/parser.py
@@ -366,10 +366,11 @@  def append(self, line):
 
         The way that the line is dealt with depends on which part of
         the documentation we're parsing right now:
-        * The body section: ._append_line is ._append_body_line
-        * An argument section: ._append_line is ._append_args_line
-        * A features section: ._append_line is ._append_features_line
-        * An additional section: ._append_line is ._append_various_line
+
+         * The body section: ._append_line is ._append_body_line
+         * An argument section: ._append_line is ._append_args_line
+         * A features section: ._append_line is ._append_features_line
+         * An additional section: ._append_line is ._append_various_line
         """
         line = line[1:]
         if not line: