Message-ID: <20180510112113.4db65764@vento.lan>
Date:   Thu, 10 May 2018 11:21:13 -0300
From:   Mauro Carvalho Chehab <mchehab+samsung@...nel.org>
To:     Jonathan Corbet <corbet@....net>
Cc:     Christoph Hellwig <hch@...radead.org>,
        Linux Doc Mailing List <linux-doc@...r.kernel.org>,
        Mauro Carvalho Chehab <mchehab@...radead.org>,
        linux-kernel@...r.kernel.org, Ingo Molnar <mingo@...hat.com>,
        Peter Zijlstra <peterz@...radead.org>
Subject: Re: [PATCH 13/18] wait: wait.h: Get rid of a kernel-doc/Sphinx
 warnings

Em Thu, 10 May 2018 07:30:12 -0600
Jonathan Corbet <corbet@....net> escreveu:

> On Thu, 10 May 2018 06:38:05 -0300
> Mauro Carvalho Chehab <mchehab+samsung@...nel.org> wrote:
> 
> > (Peter said):  
> > > Independent of any philosophical discussion not allowing a setence to
> > > end with a single ':' is completely idiotic.  Please fix the tooling
> > > instead to allow it, as it is very important for being able to just
> > > write understandable comments.    
> 
> FWIW, there's no problem with a sentence ending with a single colon.
> It's only an issue if you want to flag a special interpretation for the
> text that follows that sentence.  Just to be precise.
> 
> > Patches are welcome, although I don't see any easy way to solve it.  
> 
> I could envision some sort of heuristic that would recognize an indented
> block containing code.  Probably we could go simpler and force the
> "literal block" treatment for any indented block that lacks explicit
> enumeration markers.  So:
> 
> 	this->would_be("a literal block");
> 
> but:
> 
>   - This would not be
> 
> Such a thing would likely be a bit fragile (people feel, rightly, that
> they can put anything into normal text) but it might just work well
> enough.  For best results, it should probably be done as part of Sphinx
> itself, rather than yet another ugly hack in the kerneldoc script.

I guess that it also won't work...

$ git grep -A2 "\*\s.*\s.*:$" -- $(git grep kernel-doc:: Documentation/|cut -d : -f 4-)

Present some matches that seem to be violating such hint.

drivers/edac/edac_device.h:/* The offset value can be:
drivers/edac/edac_device.h- *   -1 indicating no offset value
drivers/edac/edac_device.h- *   0 for zero-based block numbers

drivers/gpu/drm/drm_scdc_helper.c: *    As per the spec:
drivers/gpu/drm/drm_scdc_helper.c- *            TMDS clock rate for pixel clock < 340 MHz = 1x the character
drivers/gpu/drm/drm_scdc_helper.c- *            rate = 1/10 pixel clock rate

I didn't actually check if those are part of a Kernel-doc markup, though,
but it shows that people sometimes don't add a "prepend" character to
a list.

In the specific case of errors, prepending with a "-" can be weird,
as it may lead ugly things like:

	* - -1 indicating no offset value
	* - 0 for zero-based block numbers

The problem with a hint-based mechanism is that it will generate
false hints. If added, we may end by needing to add extra tags to
disable the hints mechanism where it gets wrong, or to periodically
do code changes at kernel-doc comments in order to make the hints
logic happy.

So, IMO, we should provide non-hints based mechanism, like forcing the
string that prepends the colon to have a keyword that will make it to
parse the block as literal, where expressions like:

	See the code-block foo:
	See the following code example:
	See the following flow diagram:
	See the following artwork:

Is the best alternative to avoid "::", as on the enclosed patch.

> This particular problem may be solvable, and I'll look into it, but not
> right away.  The offline world is being rather insistently obnoxious
> these days...

Thanks,
Mauro

[PATCH] Mark a diagram at wait.h as such, using a code-block for it

Instead of explicitly using "::" to mark the diagram,
detect it based on code words inside the description.

Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@...nel.org>

diff --git a/include/linux/wait.h b/include/linux/wait.h
index d9f131ecf708..c360c5947374 100644
--- a/include/linux/wait.h
+++ b/include/linux/wait.h
@@ -101,7 +101,7 @@ init_waitqueue_func_entry(struct wait_queue_entry *wq_entry, wait_queue_func_t f
  * lead to sporadic and non-obvious failure.
  *
  * Use either while holding wait_queue_head::lock or when used for wakeups
- * with an extra smp_mb() like:
+ * with an extra smp_mb() like on this diagram:
  *
  *      CPU0 - waker                    CPU1 - waiter
  *
diff --git a/scripts/kernel-doc b/scripts/kernel-doc
index 0057d8eafcc1..1c69072997f8 100755
--- a/scripts/kernel-doc
+++ b/scripts/kernel-doc
@@ -803,7 +803,12 @@ sub output_highlight_rst {
 	#
 	if (! $in_literal) {
 	    $block .= $line . "\n";
-	    if (($line =~ /$sphinx_literal/) || ($line =~ /$sphinx_cblock/)) {
+	    if ($block =~ s/(code|example|artwork|flow|diagram)([^\:]*:)\n/$1$2:\n/) {
+		$in_literal = 1;
+		$litprefix = "";
+		$output .= highlight_block($block);
+		$block = ""
+	    } elsif (($line =~ /$sphinx_literal/) || ($line =~ /$sphinx_cblock/)) {
 		$in_literal = 1;
 		$litprefix = "";
 		$output .= highlight_block($block);

Powered by blists - more mailing lists