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

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

[Takashi Iwai]

On Sat, 26 Dec 2020 14:18:58 +0100,
Nícolas F. R. A. Prado wrote:
> 
> 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):
> 
>    `Display text <#section-name-in-html>`__
> 
> This syntax can work in html, but isn't the one described in docutils,
> 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>

For the sound part:

Reviewed-by: Takashi Iwai <tiwai@suse.de>


thanks,

Takashi

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

[Mauro Carvalho Chehab]

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

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

[Nícolas F. R. A. Prado]

On Sun Dec 27, 2020 at 6:59 AM -03, Mauro Carvalho Chehab wrote:
> 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.

Indeed, some of those references aren't even valid according to the spec, but
that isn't even the real issue. The issue is that they only work in html. They
*don't* work in the LaTeX pdf (already supported) and in the rst2pdf's pdf (not
yet supported). 

Even "#Forcing Quiescent States" only works in html, and doesn't in pdf.
"Forcing Quiescent States_", on the other hand, works in all of them. More
details below.

I should have been more clear in the commit message about that, and I will for
v2.

> > @@ -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>`__

Actually, that doesn't work. To reference a target in rst, like a section, an _
is needed at the end of the reference (and when it is a named link, this happens
inside the <>). Take a look at [1].

While reading [1], though, I noticed that it should also be possible to not
have the link anonymous, that is, to use _ instead of __ at the end. And that
fails in rst2pdf specifically, so that's an issue I'll report there.

> In this specific case, however, you could use, instead[1]:
>
> summary_
>
> as there's no need to use an indirect hyperlink target here.

You're right, this does work. I didn't realize these references were
case-insensitive. So for v2, I'll also see if there are any other references in
my patch whose text only differs the target in casing, and use this simplified
syntax for those.

Thanks,
Nícolas

[1] https://docutils.sourceforge.io/docs/ref/rst/restructuredtext.html#embedded-uris-and-aliases