2019-06-24 10:35:12

by Mauro Carvalho Chehab

[permalink] [raw]
Subject: [PATCH v2] hrtimer: Use a bullet for the returns bullet list

That gets rid of this warning:

./kernel/time/hrtimer.c:1119: WARNING: Block quote ends without a blank line; unexpected unindent.

and displays nicely both at the source code and at the produced
documentation.

Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
kernel/time/hrtimer.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/kernel/time/hrtimer.c b/kernel/time/hrtimer.c
index edb230aba3d1..5ee77f1a8a92 100644
--- a/kernel/time/hrtimer.c
+++ b/kernel/time/hrtimer.c
@@ -1114,9 +1114,10 @@ EXPORT_SYMBOL_GPL(hrtimer_start_range_ns);
* @timer: hrtimer to stop
*
* Returns:
- * 0 when the timer was not active
- * 1 when the timer was active
- * -1 when the timer is currently executing the callback function and
+ *
+ * * 0 when the timer was not active
+ * * 1 when the timer was active
+ * * -1 when the timer is currently executing the callback function and
* cannot be stopped
*/
int hrtimer_try_to_cancel(struct hrtimer *timer)
--
2.21.0



Subject: [tip:timers/core] hrtimer: Use a bullet for the returns bullet list

Commit-ID: 516337048fa40496ae5ca9863c367ec991a44d9a
Gitweb: https://git.kernel.org/tip/516337048fa40496ae5ca9863c367ec991a44d9a
Author: Mauro Carvalho Chehab <[email protected]>
AuthorDate: Mon, 24 Jun 2019 07:33:26 -0300
Committer: Thomas Gleixner <[email protected]>
CommitDate: Thu, 27 Jun 2019 23:30:04 +0200

hrtimer: Use a bullet for the returns bullet list

That gets rid of this warning:

./kernel/time/hrtimer.c:1119: WARNING: Block quote ends without a blank line; unexpected unindent.

and displays nicely both at the source code and at the produced
documentation.

Signed-off-by: Mauro Carvalho Chehab <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
Cc: Linux Doc Mailing List <[email protected]>
Cc: Mauro Carvalho Chehab <[email protected]>
Cc: Jonathan Corbet <[email protected]>
Link: https://lkml.kernel.org/r/74ddad7dac331b4e5ce4a90e15c8a49e3a16d2ac.1561372382.git.mchehab+samsung@kernel.org

---
kernel/time/hrtimer.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/kernel/time/hrtimer.c b/kernel/time/hrtimer.c
index edb230aba3d1..5ee77f1a8a92 100644
--- a/kernel/time/hrtimer.c
+++ b/kernel/time/hrtimer.c
@@ -1114,9 +1114,10 @@ EXPORT_SYMBOL_GPL(hrtimer_start_range_ns);
* @timer: hrtimer to stop
*
* Returns:
- * 0 when the timer was not active
- * 1 when the timer was active
- * -1 when the timer is currently executing the callback function and
+ *
+ * * 0 when the timer was not active
+ * * 1 when the timer was active
+ * * -1 when the timer is currently executing the callback function and
* cannot be stopped
*/
int hrtimer_try_to_cancel(struct hrtimer *timer)

2019-06-27 22:09:49

by Joe Perches

[permalink] [raw]
Subject: Re: [tip:timers/core] hrtimer: Use a bullet for the returns bullet list

On Thu, 2019-06-27 at 14:46 -0700, tip-bot for Mauro Carvalho Chehab
wrote:
> Commit-ID: 516337048fa40496ae5ca9863c367ec991a44d9a
> Gitweb: https://git.kernel.org/tip/516337048fa40496ae5ca9863c367ec991a44d9a
> Author: Mauro Carvalho Chehab <[email protected]>
> AuthorDate: Mon, 24 Jun 2019 07:33:26 -0300
> Committer: Thomas Gleixner <[email protected]>
> CommitDate: Thu, 27 Jun 2019 23:30:04 +0200
>
> hrtimer: Use a bullet for the returns bullet list
>
> That gets rid of this warning:
>
> ./kernel/time/hrtimer.c:1119: WARNING: Block quote ends without a blank line; unexpected unindent.

Doesn't this form occur multiple dozens of times in
kernel sources?

For instance:

$ git grep -B3 -A5 -P "^ \* Returns:?$" | \
grep -P -A8 '\-\s+\*\s*@\w+:'

I think the warning is odd at best and docutils might
be updated or the warning ignored or suppressed.

> and displays nicely both at the source code and at the produced
> documentation.

> diff --git a/kernel/time/hrtimer.c b/kernel/time/hrtimer.c
[]
> @@ -1114,9 +1114,10 @@ EXPORT_SYMBOL_GPL(hrtimer_start_range_ns);
> * @timer: hrtimer to stop
> *
> * Returns:
> - * 0 when the timer was not active
> - * 1 when the timer was active
> - * -1 when the timer is currently executing the callback function and
> + *
> + * * 0 when the timer was not active
> + * * 1 when the timer was active
> + * * -1 when the timer is currently executing the callback function and
> * cannot be stopped
> */
> int hrtimer_try_to_cancel(struct hrtimer *timer)

2019-06-28 00:40:45

by Mauro Carvalho Chehab

[permalink] [raw]
Subject: Re: [tip:timers/core] hrtimer: Use a bullet for the returns bullet list

Em Thu, 27 Jun 2019 15:08:59 -0700
Joe Perches <[email protected]> escreveu:

> On Thu, 2019-06-27 at 14:46 -0700, tip-bot for Mauro Carvalho Chehab
> wrote:
> > Commit-ID: 516337048fa40496ae5ca9863c367ec991a44d9a
> > Gitweb: https://git.kernel.org/tip/516337048fa40496ae5ca9863c367ec991a44d9a
> > Author: Mauro Carvalho Chehab <[email protected]>
> > AuthorDate: Mon, 24 Jun 2019 07:33:26 -0300
> > Committer: Thomas Gleixner <[email protected]>
> > CommitDate: Thu, 27 Jun 2019 23:30:04 +0200
> >
> > hrtimer: Use a bullet for the returns bullet list
> >
> > That gets rid of this warning:
> >
> > ./kernel/time/hrtimer.c:1119: WARNING: Block quote ends without a blank line; unexpected unindent.
>
> Doesn't this form occur multiple dozens of times in
> kernel sources?
>
> For instance:
>
> $ git grep -B3 -A5 -P "^ \* Returns:?$" | \
> grep -P -A8 '\-\s+\*\s*@\w+:'

Yes, this is a common pattern, but not all patterns that match the above
regex are broken.

>
> I think the warning is odd at best and docutils might
> be updated or the warning ignored or suppressed.
>
> > and displays nicely both at the source code and at the produced
> > documentation.

The warnings are painful - and they're the main reason why I wrote this
change: - I wanted to avoid new warnings actually unrelated to my
changes that were sometimes appearing while doing incremental
"make htmldocs" on a big patchset that I've been rebasing almost every
week over the last two months.

-

Yet, did you try to look how this pattern will appear at the html and pdf
output? Something like this:

sound/soc/codecs/wm8960.c: * Returns:
sound/soc/codecs/wm8960.c- * -1, in case no sysclk frequency available found
sound/soc/codecs/wm8960.c- * >=0, in case we could derive bclk and lrclk from sysclk using
sound/soc/codecs/wm8960.c- * (@sysclk_idx, @dac_idx, @bclk_idx) dividers


Will be displayed as:

**Returns:**
-1, in case no sysclk frequency available found **>=0, in case we could derive bclk and lrclk from sysclk using** (@sysclk_idx, @dac_idx, @bclk_idx) dividers


(where **foo**) means that "foo" will be printed in bold.

E. g. it will just merge all returns values into a single line and, if
there are alignment differences, it will make the previous line bold
and produce a warning.

On some places, however, what's there will be properly displayed,
like this one:
**
* wimax_reset - Reset a WiMAX device
*
* @wimax_dev: WiMAX device descriptor
*
* Returns:
*
* %0 if ok and a warm reset was done (the device still exists in
* the system).
*
* -%ENODEV if a cold/bus reset had to be done (device has
* disconnected and reconnected, so current handle is not valid
* any more).
*
* -%EINVAL if the device is not even registered.
*
* Any other negative error code shall be considered as
* non-recoverable.
*

As there are blank lines between each value, making each return code a
different line.

This one:

tools/lib/traceevent/parse-filter.c: * Returns:
tools/lib/traceevent/parse-filter.c- * 1 if the two filters hold the same content.
tools/lib/traceevent/parse-filter.c- * 0 if they do not.

will also not mangle too much, as the dots will help for someone to
understand, if reading the html/pdf output, like this:

**Returns:**
1 if the two filters hold the same content. 0 if they do not.

So, it all depends on the context.

-

While it would likely be possible to improve kernel-doc to present better
results, I'm afraid that it would be too complex for simple regex
expressions, and hard to tune, as it would be a hint-based approach,
and doing a natural language processing would be too much effort.


>
> > diff --git a/kernel/time/hrtimer.c b/kernel/time/hrtimer.c
> []
> > @@ -1114,9 +1114,10 @@ EXPORT_SYMBOL_GPL(hrtimer_start_range_ns);
> > * @timer: hrtimer to stop
> > *
> > * Returns:
> > - * 0 when the timer was not active
> > - * 1 when the timer was active
> > - * -1 when the timer is currently executing the callback function and
> > + *
> > + * * 0 when the timer was not active
> > + * * 1 when the timer was active
> > + * * -1 when the timer is currently executing the callback function and
> > * cannot be stopped
> > */
> > int hrtimer_try_to_cancel(struct hrtimer *timer)
>



Thanks,
Mauro

2019-06-28 02:41:14

by Joe Perches

[permalink] [raw]
Subject: Re: [tip:timers/core] hrtimer: Use a bullet for the returns bullet list

On Thu, 2019-06-27 at 21:39 -0300, Mauro Carvalho Chehab wrote:
> Em Thu, 27 Jun 2019 15:08:59 -0700
> Joe Perches <[email protected]> escreveu:
[]
> > > hrtimer: Use a bullet for the returns bullet list
> > >
> > > That gets rid of this warning:
> > >
> > > ./kernel/time/hrtimer.c:1119: WARNING: Block quote ends without a blank line; unexpected unindent.
> >
> > Doesn't this form occur multiple dozens of times in
> > kernel sources?
> >
> > For instance:
> >
> > $ git grep -B3 -A5 -P "^ \* Returns:?$" | \
> > grep -P -A8 '\-\s+\*\s*@\w+:'
>
> Yes, this is a common pattern, but not all patterns that match the above
> regex are broken.
>
> > I think the warning is odd at best and docutils might
> > be updated or the warning ignored or suppressed.
> >
> > > and displays nicely both at the source code and at the produced
> > > documentation.
>
> The warnings are painful - and they're the main reason why I wrote this
> change: - I wanted to avoid new warnings actually unrelated to my
> changes that were sometimes appearing while doing incremental
> "make htmldocs" on a big patchset that I've been rebasing almost every
> week over the last two months.
>
> -
>
> Yet, did you try to look how this pattern will appear at the html and pdf
> output?

No I did not.

I just would like to avoid changing perfectly intelligible
kernel-doc content into something less directly readable for
the sake of external output.

I don't use the externally generated formatted output docs.
I read and use the source when necessary.

Automatic creation of bulleted blocks from relatively
unformatted content is a hard problem.

I appreciate the work Mauro, I just would like to minimize
the necessary changes if possible.

The grep I did was trivial, I'm sure there are better tools
to isolate the kernel-doc bits where the Return: block
is emitted.


> Something like this:
>
> sound/soc/codecs/wm8960.c: * Returns:
> sound/soc/codecs/wm8960.c- * -1, in case no sysclk frequency available found
> sound/soc/codecs/wm8960.c- * >=0, in case we could derive bclk and lrclk from sysclk using
> sound/soc/codecs/wm8960.c- * (@sysclk_idx, @dac_idx, @bclk_idx) dividers
>
>
> Will be displayed as:
>
> **Returns:**
> -1, in case no sysclk frequency available found **>=0, in case we could derive bclk and lrclk from sysclk using** (@sysclk_idx, @dac_idx, @bclk_idx) dividers
> (where **foo**) means that "foo" will be printed in bold.>

That's a yuck from me.

> While it would likely be possible to improve kernel-doc to present better
> results, I'm afraid that it would be too complex for simple regex
> expressions, and hard to tune, as it would be a hint-based approach,
> and doing a natural language processing would be too much effort.

Yeah, tough problem. I don't envy it.

cheers and g'luck...

2019-06-28 13:13:07

by Mauro Carvalho Chehab

[permalink] [raw]
Subject: Re: [tip:timers/core] hrtimer: Use a bullet for the returns bullet list

Em Thu, 27 Jun 2019 19:40:24 -0700
Joe Perches <[email protected]> escreveu:

> On Thu, 2019-06-27 at 21:39 -0300, Mauro Carvalho Chehab wrote:
> > Em Thu, 27 Jun 2019 15:08:59 -0700
> > Joe Perches <[email protected]> escreveu:
> []
> > > > hrtimer: Use a bullet for the returns bullet list
> > > >
> > > > That gets rid of this warning:
> > > >
> > > > ./kernel/time/hrtimer.c:1119: WARNING: Block quote ends without a blank line; unexpected unindent.
> > >
> > > Doesn't this form occur multiple dozens of times in
> > > kernel sources?
> > >
> > > For instance:
> > >
> > > $ git grep -B3 -A5 -P "^ \* Returns:?$" | \
> > > grep -P -A8 '\-\s+\*\s*@\w+:'
> >
> > Yes, this is a common pattern, but not all patterns that match the above
> > regex are broken.
> >
> > > I think the warning is odd at best and docutils might
> > > be updated or the warning ignored or suppressed.
> > >
> > > > and displays nicely both at the source code and at the produced
> > > > documentation.
> >
> > The warnings are painful - and they're the main reason why I wrote this
> > change: - I wanted to avoid new warnings actually unrelated to my
> > changes that were sometimes appearing while doing incremental
> > "make htmldocs" on a big patchset that I've been rebasing almost every
> > week over the last two months.
> >
> > -
> >
> > Yet, did you try to look how this pattern will appear at the html and pdf
> > output?
>
> No I did not.
>
> I just would like to avoid changing perfectly intelligible
> kernel-doc content into something less directly readable for
> the sake of external output.
>
> I don't use the externally generated formatted output docs.
> I read and use the source when necessary.

Yeah, I do the same too, except when I want to se the hole
picture or on complex subsystems. You can't really understand
media subsystems if you don't read it from the docs, as
the rationale for a lot of things are there.

Yet, for newcomers that are studying a new subsystem, a
good documentation helps a lot.

>
> Automatic creation of bulleted blocks from relatively
> unformatted content is a hard problem.
>
> I appreciate the work Mauro, I just would like to minimize
> the necessary changes if possible.

Yeah, me too.

>
> The grep I did was trivial, I'm sure there are better tools
> to isolate the kernel-doc bits where the Return: block
> is emitted.
>
>
> > Something like this:
> >
> > sound/soc/codecs/wm8960.c: * Returns:
> > sound/soc/codecs/wm8960.c- * -1, in case no sysclk frequency available found
> > sound/soc/codecs/wm8960.c- * >=0, in case we could derive bclk and lrclk from sysclk using
> > sound/soc/codecs/wm8960.c- * (@sysclk_idx, @dac_idx, @bclk_idx) dividers
> >
> >
> > Will be displayed as:
> >
> > **Returns:**
> > -1, in case no sysclk frequency available found **>=0, in case we could derive bclk and lrclk from sysclk using** (@sysclk_idx, @dac_idx, @bclk_idx) dividers
> > (where **foo**) means that "foo" will be printed in bold.>
>
> That's a yuck from me.

Agreed, but, when writing text docs, doing something like:

parameter
parameter description

and getting the first line bolded helps to reduce the need of
adding explicit bold markups and produce a nice visual.

>
> > While it would likely be possible to improve kernel-doc to present better
> > results, I'm afraid that it would be too complex for simple regex
> > expressions, and hard to tune, as it would be a hint-based approach,
> > and doing a natural language processing would be too much effort.
>
> Yeah, tough problem. I don't envy it.
>
> cheers and g'luck...

Thank you!

Thanks,
Mauro