Re: [PATCH 00/34] docs: avoid using ReST :doc:`foo` tag

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

On Sat, Jun 05, 2021 at 09:08:36PM +0200, Mauro Carvalho Chehab wrote:
> Em Sat, 5 Jun 2021 12:11:09 -0300
> Nícolas F. R. A. Prado <n@xxxxxxxxxxxxx> escreveu:
> 
> > Hi Mauro,
> > 
> > On Sat, Jun 05, 2021 at 03:17:59PM +0200, Mauro Carvalho Chehab wrote:
> > > As discussed at:
> > > 	https://lore.kernel.org/linux-doc/871r9k6rmy.fsf@xxxxxxxxxxxx/
> > > 
> > > It is better to avoid using :doc:`foo` to refer to Documentation/foo.rst, as the
> > > automarkup.py extension should handle it automatically, on most cases.
> > > 
> > > There are a couple of exceptions to this rule:
> > > 
> > > 1. when :doc:  tag is used to point to a kernel-doc DOC: markup;
> > > 2. when it is used with a named tag, e. g. :doc:`some name <foo>`;
> > > 
> > > It should also be noticed that automarkup.py has currently an issue:
> > > if one use a markup like:
> > > 
> > > 	Documentation/dev-tools/kunit/api/test.rst
> > > 	  - documents all of the standard testing API excluding mocking
> > > 	    or mocking related features.
> > > 
> > > or, even:
> > > 
> > > 	Documentation/dev-tools/kunit/api/test.rst
> > > 	    documents all of the standard testing API excluding mocking
> > > 	    or mocking related features.
> > > 	
> > > The automarkup.py will simply ignore it. Not sure why. This patch series
> > > avoid the above patterns (which is present only on 4 files), but it would be
> > > nice to have a followup patch fixing the issue at automarkup.py.  
> > 
> > What I think is happening here is that we're using rST's syntax for definition
> > lists [1]. automarkup.py ignores literal nodes, and perhaps a definition is
> > considered a literal by Sphinx. Adding a blank line after the Documentation/...
> > or removing the additional indentation makes it work, like you did in your
> > 2nd and 3rd patch, since then it's not a definition anymore, although then the
> > visual output is different as well.
> 
> A literal has a different output. I think that this is not the case, but I 
> didn't check the python code from docutils/Sphinx.

Okay, I went in deeper to understand the issue and indeed it wasn't what I
thought. The reason definitions are ignored by automarkup.py is because the main
loop iterates only over nodes that are of type paragraph:

    for para in doctree.traverse(nodes.paragraph):
        for node in para.traverse(nodes.Text):
            if not isinstance(node.parent, nodes.literal):
                node.parent.replace(node, markup_refs(name, app, node))

And inspecting the HTML output from your example, the definition name is inside
a <dt> tag, and it doesn't have a <p> inside. So in summary, automarkup.py will
only work on elements which are inside a <p> in the output.

Only applying the automarkup inside paragraphs seems like a good decision (which
covers text in lists and tables as well), so unless there are other types of
elements without paragraphs where automarkup should work, I think we should just
avoid using definition lists pointing to documents like that.

>  
> > I'm not sure this is something we need to fix. Does it make sense to use
> > definition lists for links like that? If it does, I guess one option would be to
> > whitelist definition lists so they aren't ignored by automarkup, but I feel
> > this could get ugly really quickly.
> 
> Yes, we should avoid handling literal blocks, as this can be a nightmare.
> 
> > FWIW note that it's also possible to use relative paths to docs with automarkup.
> 
> Not sure if you meant to say using something like ../driver-api/foo.rst.
> If so, relative paths are a problem, as it will pass unnoticed by this script:
> 
> 	./scripts/documentation-file-ref-check
> 
> which is meant to warn when a file is moved to be elsewhere. Ok, it
> could be taught to use "../" to identify paths, but I suspect that this
> could lead to false positives, like here:
> 
> 	Documentation/usb/gadget-testing.rst:  # ln -s ../../uncompressed/u
> 	Documentation/usb/gadget-testing.rst:  # cd ../../class/fs
> 	Documentation/usb/gadget-testing.rst:  # ln -s ../../header/h

Yes, that's what I meant. 

Ok, that makes sense. Although after automarkup.py starts printing warnings on
missing references to files (which is a patch I still need to resend), it would
work out-of-the-box with relative paths. automarkup wouldn't face that false
positives issue since it ignores literal blocks, which isn't as easy for a
standalone script. But that's still in the future, we can discuss what to do
then after it is implemented, so full paths seem better for now.

Thanks,
Nícolas

> 
> If you meant, instead, :doc:`../foo`, this series address those too.
> 
> Regards,
> Mauro