Re: [PATCH] docs: Fix reST markup when linking to sections

[Mauro Carvalho Chehab <mchehab+huawei@kernel.org>]

Em Sat, 26 Dec 2020 13:18:58 +0000
Nícolas F. R. A. Prado <nfraprado@protonmail.com> escreveu:

> During the process of converting the documentation to reST, some links
> were converted using the following wrong syntax (and sometimes using %20
> instead of spaces):
> 

The patch itself looks ok, although the description has an issue (IMHO).
Also, some references can be cleaned.

See below.

>    `Display text <#section-name-in-html>`__
> 
> This syntax can work in html, but isn't the one described in docutils,

Well, docutils define two types of references at:

	https://docutils.sourceforge.io/docs/ref/rst/restructuredtext.html#reference-names

The first one are "simple reference names", defined as:

	``Simple reference names are single words consisting of
	  alphanumerics plus isolated (no two adjacent) internal
	  hyphens, underscores, periods, colons and plus signs;
	  no whitespace or other characters are allowed.``

On this type, "-_.,+" characters are allowed. "%" and "#" aren't.

The second one is "phrase-references", defined as:

	``Reference names using punctuation or whose names are phrases (two or more space-separated words) are called "phrase-references".``

Here, the spec doesn't specify the charset associated with "punctuation".

As this kind of reference is auto-generated from the chapter titles,
I would expect it to allow all ASCII punctuation characters
(e. g. all non-alphanumeric symbols between 0x20-0x7f).

So, a reference like those:
	#Summary
	#Forcing%20Quiescent%20States

Violates the spec, as it would be a simple reference with invalid
chars, but:

	#Forcing Quiescent States

Should be valid, according with the spec (still, while doing such
cleanup, I would remove "#").

I would add something like the above at the patch description.

> and it also doesn't work on pdf. The following syntax should be used
> instead:
> 
>    `Display text <Section Name_>`__
> 
> The usual toolchain doesn't mind this unusual syntax, but it causes
> errors when trying to build using the not-yet-merged rst2pdf:
> 
>    ValueError: format not resolved, probably missing URL scheme or undefined destination target for 'Forcing%20Quiescent%20States'
> 
> Fixes: ccc9971e2147 ("docs: rcu: convert some articles from html to ReST")
> Fixes: c8cce10a62aa ("docs: Fix the reference labels in Locking.rst")
> Fixes: e548cdeffcd8 ("docs-rst: convert kernel-locking to ReST")
> Fixes: 7ddedebb03b7 ("ALSA: doc: ReSTize writing-an-alsa-driver document")
> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@protonmail.com>

...

> @@ -596,7 +596,7 @@ maintain ordering. For example, if the callback function wakes up a task
>  that runs on some other CPU, proper ordering must in place in both the
>  callback function and the task being awakened. To see why this is
>  important, consider the top half of the `grace-period
> -cleanup <#Grace-Period%20Cleanup>`__ diagram. The callback might be
> +cleanup <Grace-Period Cleanup_>`__ diagram. The callback might be
>  running on a CPU corresponding to the leftmost leaf ``rcu_node``
>  structure, and awaken a task that is to run on a CPU corresponding to
>  the rightmost leaf ``rcu_node`` structure, and the grace-period kernel
> diff --git a/Documentation/RCU/Design/Requirements/Requirements.rst b/Documentation/RCU/Design/Requirements/Requirements.rst
> index 1ae79a10a8de..d4125caf394e 100644
> --- a/Documentation/RCU/Design/Requirements/Requirements.rst
> +++ b/Documentation/RCU/Design/Requirements/Requirements.rst
> @@ -45,7 +45,7 @@ requirements:
>  #. `Other RCU Flavors`_
>  #. `Possible Future Changes`_
>  
> -This is followed by a `summary <#Summary>`__, however, the answers to
> +This is followed by a `summary <Summary_>`__, however, the answers to

Hmm... why are you ending "Summary" with a "_"? This should be
equivalent to:

	`summary <summary>`__

In this specific case, however, you could use, instead[1]:

	summary_

as there's no need to use an indirect hyperlink target here.

(the same applies to a few other similar cases on your patch)

[1] https://docutils.sourceforge.io/docs/user/rst/quickref.html#hyperlink-targets

Thanks,
Mauro