| Message ID | cover.1622898327.git.mchehab+huawei@kernel.org |
|---|---|
| Headers | show |
| Series | docs: avoid using ReST :doc:`foo` tag | expand |
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@nfraprado.net> 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@meer.lwn.net/ > > > > > > 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
Em Sun, 6 Jun 2021 19:52:25 -0300 Nícolas F. R. A. Prado <n@nfraprado.net> escreveu: > 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@nfraprado.net> 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@meer.lwn.net/ > > > > > > > > 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. Yeah, that's what I was suspecting, based on the comments. Maybe something similar to the above could be done also for some non-paragraph data. By looking at: https://docutils.sourceforge.io/docs/ref/doctree.html It says that the body elements are: admonition, attention, block_quote, bullet_list, caution, citation, comment, compound, container, danger, definition_list, doctest_block, enumerated_list, error, field_list, figure, footnote, hint, image, important, line_block, literal_block, note, option_list, paragraph, pending, raw, rubric, substitution_definition, system_message, table, target, tip, warning So, perhaps a similar loop for definition_list would do the trick, but maybe automarkup should also look at other types, like enum lists, notes (and their variants, like error/warning) and footnotes. No idea how this would affect the docs build time, though. > 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. Checking the code or doing some tests are needed for us to be sure about what of the above types docutils don't consider a paragraph. Thanks, Mauro
As discussed at: https://lore.kernel.org/linux-doc/871r9k6rmy.fsf@meer.lwn.net/ 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. On this series: Patch 1 manually adjust the references inside driver-api/pm/devices.rst, as there it uses :file:`foo` to refer to some Documentation/ files; Patch 2 converts a table at Documentation/dev-tools/kunit/api/index.rst into a list, carefully avoiding the Patch 3 converts the cross-references at the media documentation, also avoiding the automarkup.py bug; Patches 4-34 convert the other occurrences via a replace script. They were manually edited, in order to honour 80-columns where possible. I did a diff between the Sphinx 2.4.4 output before and after this patch series in order to double-check that all converted Documentation/ references will produce <a href=<foo>.rst>foo title</a> tags. Mauro Carvalho Chehab (34): docs: devices.rst: better reference documentation docs docs: dev-tools: kunit: don't use a table for docs name media: docs: */media/index.rst: don't use ReST doc:`foo` media: userspace-api: avoid using ReST :doc:`foo` markup media: driver-api: drivers: avoid using ReST :doc:`foo` markup media: admin-guide: avoid using ReST :doc:`foo` markup docs: admin-guide: pm: avoid using ReSt :doc:`foo` markup docs: admin-guide: hw-vuln: avoid using ReST :doc:`foo` markup docs: admin-guide: sysctl: avoid using ReST :doc:`foo` markup docs: block: biodoc.rst: avoid using ReSt :doc:`foo` markup docs: bpf: bpf_lsm.rst: avoid using ReSt :doc:`foo` markup docs: core-api: avoid using ReSt :doc:`foo` markup docs: dev-tools: testing-overview.rst: avoid using ReSt :doc:`foo` markup docs: dev-tools: kunit: avoid using ReST :doc:`foo` markup docs: devicetree: bindings: submitting-patches.rst: avoid using ReSt :doc:`foo` markup docs: doc-guide: avoid using ReSt :doc:`foo` markup docs: driver-api: avoid using ReSt :doc:`foo` markup docs: driver-api: gpio: using-gpio.rst: avoid using ReSt :doc:`foo` markup docs: driver-api: surface_aggregator: avoid using ReSt :doc:`foo` markup docs: driver-api: usb: avoid using ReSt :doc:`foo` markup docs: firmware-guide: acpi: avoid using ReSt :doc:`foo` markup docs: hwmon: adm1177.rst: avoid using ReSt :doc:`foo` markup docs: i2c: avoid using ReSt :doc:`foo` markup docs: kernel-hacking: hacking.rst: avoid using ReSt :doc:`foo` markup docs: networking: devlink: avoid using ReSt :doc:`foo` markup docs: PCI: endpoint: pci-endpoint-cfs.rst: avoid using ReSt :doc:`foo` markup docs: PCI: pci.rst: avoid using ReSt :doc:`foo` markup docs: process: submitting-patches.rst: avoid using ReSt :doc:`foo` markup docs: security: landlock.rst: avoid using ReSt :doc:`foo` markup docs: trace: coresight: coresight.rst: avoid using ReSt :doc:`foo` markup docs: trace: ftrace.rst: avoid using ReSt :doc:`foo` markup docs: userspace-api: landlock.rst: avoid using ReSt :doc:`foo` markup docs: virt: kvm: s390-pv-boot.rst: avoid using ReSt :doc:`foo` markup docs: x86: avoid using ReSt :doc:`foo` markup .../PCI/endpoint/pci-endpoint-cfs.rst | 2 +- Documentation/PCI/pci.rst | 6 +-- .../special-register-buffer-data-sampling.rst | 3 +- Documentation/admin-guide/media/bt8xx.rst | 15 ++++---- Documentation/admin-guide/media/bttv.rst | 21 ++++++----- Documentation/admin-guide/media/index.rst | 12 +++--- Documentation/admin-guide/media/saa7134.rst | 3 +- Documentation/admin-guide/pm/intel_idle.rst | 16 +++++--- Documentation/admin-guide/pm/intel_pstate.rst | 9 +++-- Documentation/admin-guide/sysctl/abi.rst | 2 +- Documentation/admin-guide/sysctl/kernel.rst | 37 ++++++++++--------- Documentation/block/biodoc.rst | 2 +- Documentation/bpf/bpf_lsm.rst | 13 ++++--- .../core-api/bus-virt-phys-mapping.rst | 2 +- Documentation/core-api/dma-api.rst | 5 ++- Documentation/core-api/dma-isa-lpc.rst | 2 +- Documentation/core-api/index.rst | 4 +- Documentation/dev-tools/kunit/api/index.rst | 8 ++-- Documentation/dev-tools/kunit/faq.rst | 2 +- Documentation/dev-tools/kunit/index.rst | 14 +++---- Documentation/dev-tools/kunit/start.rst | 6 +-- Documentation/dev-tools/kunit/tips.rst | 5 ++- Documentation/dev-tools/kunit/usage.rst | 8 ++-- Documentation/dev-tools/testing-overview.rst | 16 ++++---- .../bindings/submitting-patches.rst | 11 +++--- Documentation/doc-guide/contributing.rst | 8 ++-- Documentation/driver-api/gpio/using-gpio.rst | 4 +- Documentation/driver-api/ioctl.rst | 2 +- .../driver-api/media/drivers/bttv-devel.rst | 2 +- Documentation/driver-api/media/index.rst | 10 +++-- Documentation/driver-api/pm/devices.rst | 8 ++-- .../surface_aggregator/clients/index.rst | 3 +- .../surface_aggregator/internal.rst | 15 ++++---- .../surface_aggregator/overview.rst | 6 ++- Documentation/driver-api/usb/dma.rst | 6 +-- .../acpi/dsd/data-node-references.rst | 3 +- .../firmware-guide/acpi/dsd/graph.rst | 2 +- .../firmware-guide/acpi/enumeration.rst | 7 ++-- Documentation/hwmon/adm1177.rst | 3 +- Documentation/i2c/instantiating-devices.rst | 2 +- Documentation/i2c/old-module-parameters.rst | 3 +- Documentation/i2c/smbus-protocol.rst | 4 +- Documentation/kernel-hacking/hacking.rst | 4 +- .../networking/devlink/devlink-region.rst | 2 +- .../networking/devlink/devlink-trap.rst | 4 +- Documentation/process/submitting-patches.rst | 32 ++++++++-------- Documentation/security/landlock.rst | 3 +- Documentation/trace/coresight/coresight.rst | 8 ++-- Documentation/trace/ftrace.rst | 2 +- Documentation/userspace-api/landlock.rst | 11 +++--- .../userspace-api/media/glossary.rst | 2 +- Documentation/userspace-api/media/index.rst | 12 +++--- Documentation/virt/kvm/s390-pv-boot.rst | 2 +- Documentation/x86/boot.rst | 4 +- Documentation/x86/mtrr.rst | 2 +- 55 files changed, 217 insertions(+), 183 deletions(-)