[PATCH] kernel-doc: Make the description of return value readable

[PATCH] kernel-doc: Make the description of return value readable

[Jarkko Sakkinen]

The description on how to describe return values is over-complicated, and
hard to follow. For alien reason, the body of the section is a note, and
the first paragraph speaks about 'Return', albeit the section name is
actually 'Return:'.

To give a better help when both implementing and reviewing patches, provide
a straight-forward guideline, how to decribe return values, instead of
providing a note that starts by "blacklisting" one of the infinite possible
options of doing it wrong.

This decreases the cumulative amount of time, which is probably
substantial, on this otherwise somewhat trivial topic.

Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
 Documentation/doc-guide/kernel-doc.rst | 34 +++++++-------------------
 1 file changed, 9 insertions(+), 25 deletions(-)

diff --git a/Documentation/doc-guide/kernel-doc.rst b/Documentation/doc-guide/kernel-doc.rst
index 79aaa55d6bcf..dc5e1722c150 100644
--- a/Documentation/doc-guide/kernel-doc.rst
+++ b/Documentation/doc-guide/kernel-doc.rst
@@ -136,34 +136,18 @@ Examples::
 Return values
 ~~~~~~~~~~~~~
 
-The return value, if any, should be described in a dedicated section
-named ``Return``.
+The return value, if any, should be described in a dedicated section named
+``Return:``.
 
-.. note::
-
-  #) The multi-line descriptive text you provide does *not* recognize
-     line breaks, so if you try to format some text nicely, as in::
-
-	* Return:
-	* 0 - OK
-	* -EINVAL - invalid argument
-	* -ENOMEM - out of memory
-
-     this will all run together and produce::
-
-	Return: 0 - OK -EINVAL - invalid argument -ENOMEM - out of memory
-
-     So, in order to produce the desired line breaks, you need to use a
-     ReST list, e. g.::
+In order to describe multiple return values, a ReST list should be used. That
+way Sphinx knows how to line-up them properly::
 
-      * Return:
-      * * 0		- OK to runtime suspend the device
-      * * -EBUSY	- Device should not be runtime suspended
+  * Return:
+  * - 0:	The run-time is allowed to suspend the device.
+  * - -EBUSY:	Device should not be suspended.
 
-  #) If the descriptive text you provide has lines that begin with
-     some phrase followed by a colon, each of those phrases will be taken
-     as a new section heading, which probably won't produce the desired
-     effect.
+Using a dash rather than asterisk an is probably a better idea, because it does
+not meddle as much with the C-comments.
 
 Structure, union, and enumeration documentation
 -----------------------------------------------
-- 
2.30.0

Re: [PATCH] kernel-doc: Make the description of return value readable

[Mauro Carvalho Chehab]

Em Mon, 25 Jan 2021 10:19:04 +0200
Jarkko Sakkinen <jarkko@kernel.org> escreveu:

> The description on how to describe return values is over-complicated, and
> hard to follow. For alien reason, the body of the section is a note, and
> the first paragraph speaks about 'Return', albeit the section name is
> actually 'Return:'.
> 
> To give a better help when both implementing and reviewing patches, provide
> a straight-forward guideline, how to decribe return values, instead of
> providing a note that starts by "blacklisting" one of the infinite possible
> options of doing it wrong.
> 
> This decreases the cumulative amount of time, which is probably
> substantial, on this otherwise somewhat trivial topic.
> 
> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> ---
>  Documentation/doc-guide/kernel-doc.rst | 34 +++++++-------------------
>  1 file changed, 9 insertions(+), 25 deletions(-)
> 
> diff --git a/Documentation/doc-guide/kernel-doc.rst b/Documentation/doc-guide/kernel-doc.rst
> index 79aaa55d6bcf..dc5e1722c150 100644
> --- a/Documentation/doc-guide/kernel-doc.rst
> +++ b/Documentation/doc-guide/kernel-doc.rst
> @@ -136,34 +136,18 @@ Examples::
>  Return values
>  ~~~~~~~~~~~~~
>  
> -The return value, if any, should be described in a dedicated section
> -named ``Return``.
> +The return value, if any, should be described in a dedicated section named
> +``Return:``.
>  
> -.. note::
> -
> -  #) The multi-line descriptive text you provide does *not* recognize
> -     line breaks, so if you try to format some text nicely, as in::
> -
> -	* Return:
> -	* 0 - OK
> -	* -EINVAL - invalid argument
> -	* -ENOMEM - out of memory
> -
> -     this will all run together and produce::
> -
> -	Return: 0 - OK -EINVAL - invalid argument -ENOMEM - out of memory
> -
> -     So, in order to produce the desired line breaks, you need to use a
> -     ReST list, e. g.::
> +In order to describe multiple return values, a ReST list should be used. That
> +way Sphinx knows how to line-up them properly::
>  
> -      * Return:
> -      * * 0		- OK to runtime suspend the device
> -      * * -EBUSY	- Device should not be runtime suspended
> +  * Return:
> +  * - 0:	The run-time is allowed to suspend the device.
> +  * - -EBUSY:	Device should not be suspended.
>  
> -  #) If the descriptive text you provide has lines that begin with
> -     some phrase followed by a colon, each of those phrases will be taken
> -     as a new section heading, which probably won't produce the desired
> -     effect.
> +Using a dash rather than asterisk an is probably a better idea, because it does
> +not meddle as much with the C-comments.

Well, there are different opinions with that regards... Some devs think that
using:

	- -EBUSY

is confusing. I ended agreeing with that. That's why the recommendation is
to use an asterisk.

Thanks,
Mauro

Re: [PATCH] kernel-doc: Make the description of return value readable

[Jarkko Sakkinen]

On Mon, Jan 25, 2021 at 10:53:53AM +0100, Mauro Carvalho Chehab wrote:
> Em Mon, 25 Jan 2021 10:19:04 +0200
> Jarkko Sakkinen <jarkko@kernel.org> escreveu:
> 
> > The description on how to describe return values is over-complicated, and
> > hard to follow. For alien reason, the body of the section is a note, and
> > the first paragraph speaks about 'Return', albeit the section name is
> > actually 'Return:'.
> > 
> > To give a better help when both implementing and reviewing patches, provide
> > a straight-forward guideline, how to decribe return values, instead of
> > providing a note that starts by "blacklisting" one of the infinite possible
> > options of doing it wrong.
> > 
> > This decreases the cumulative amount of time, which is probably
> > substantial, on this otherwise somewhat trivial topic.
> > 
> > Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> > ---
> >  Documentation/doc-guide/kernel-doc.rst | 34 +++++++-------------------
> >  1 file changed, 9 insertions(+), 25 deletions(-)
> > 
> > diff --git a/Documentation/doc-guide/kernel-doc.rst b/Documentation/doc-guide/kernel-doc.rst
> > index 79aaa55d6bcf..dc5e1722c150 100644
> > --- a/Documentation/doc-guide/kernel-doc.rst
> > +++ b/Documentation/doc-guide/kernel-doc.rst
> > @@ -136,34 +136,18 @@ Examples::
> >  Return values
> >  ~~~~~~~~~~~~~
> >  
> > -The return value, if any, should be described in a dedicated section
> > -named ``Return``.
> > +The return value, if any, should be described in a dedicated section named
> > +``Return:``.
> >  
> > -.. note::
> > -
> > -  #) The multi-line descriptive text you provide does *not* recognize
> > -     line breaks, so if you try to format some text nicely, as in::
> > -
> > -	* Return:
> > -	* 0 - OK
> > -	* -EINVAL - invalid argument
> > -	* -ENOMEM - out of memory
> > -
> > -     this will all run together and produce::
> > -
> > -	Return: 0 - OK -EINVAL - invalid argument -ENOMEM - out of memory
> > -
> > -     So, in order to produce the desired line breaks, you need to use a
> > -     ReST list, e. g.::
> > +In order to describe multiple return values, a ReST list should be used. That
> > +way Sphinx knows how to line-up them properly::
> >  
> > -      * Return:
> > -      * * 0		- OK to runtime suspend the device
> > -      * * -EBUSY	- Device should not be runtime suspended
> > +  * Return:
> > +  * - 0:	The run-time is allowed to suspend the device.
> > +  * - -EBUSY:	Device should not be suspended.
> >  
> > -  #) If the descriptive text you provide has lines that begin with
> > -     some phrase followed by a colon, each of those phrases will be taken
> > -     as a new section heading, which probably won't produce the desired
> > -     effect.
> > +Using a dash rather than asterisk an is probably a better idea, because it does
> > +not meddle as much with the C-comments.
> 
> Well, there are different opinions with that regards... Some devs think that
> using:
> 
> 	- -EBUSY
> 
> is confusing. I ended agreeing with that. That's why the recommendation is
> to use an asterisk.

Maybe then whitelisting two options would be the way to move forward?

> Thanks,
> Mauro

/Jarkko