Only create a target for object descriptions #10478
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
AA-Turner
requested changes
May 27, 2022
|
You also need to fix the existing test failures. A |
latosha-maltba
force-pushed
the
object_description_run_5.x
branch
5 times, most recently
from
fa8ec4a
to
800231b
Compare
|
So, I added documentation and unit tests. I'm still not quiet happy with the naming of the option: |
jakobandersen
requested changes
Jun 3, 2022
latosha-maltba
force-pushed
the
object_description_run_5.x
branch
3 times, most recently
from
eaaf978
to
e23f883
Compare
Add option :hidden: to ObjectDescription. Currently, this option has no effect except being stored as attribute in the resulting node.
Do not produce any output only a target node if option :hidden: is given. In this case all nodes representing this ObjectDescription are replaced by a single target where the target gets assigned all ids of the replaced nodes.
Record the source information (class attributes source and line) when replacing an object description node with a target node.
The collect_ids() function uses the its second parameter ``ids: List[str]`` as in- and output parameter and always returns None. Rewrite the function to become pure and side effect free: return the list of ids as the return value not as an output parameter.
replace_node_with_target() does not access the self-instance. Thus, declare it as a static method.
Move the collect_ids() method from inside the replace_node_with_target() into the class scope (as static method) since it does not need the closure of the surrounding replace_node_with_target() method. This also gives a potential speed boost.
Inline the one-line method replace_node_with_target() into its only call site. Since there are no call sites left, remove the method afterwards.
Instead of overriding the `option_spec` variable copy the `option_spec` of the superclass and update it. This way changes in the superclass propagate to its children. This is in line how other classes use "`option_spec`-inheritance".
The alias objects override the run() method and thus do not support ``:notypesetting:`` option.
Correct node order generated by the .. js:module:: directive: When generating an index entry, the index node must come before the target node.
Correct node order generated by the .. py:module:: directive: When generating an index entry, the index node must come before the target node.
It might be that a object description has no ids associated with it, e.g. if the option :noindex: is passed. In this case we would replace the object description node with a target node, which has no ids. This breaks docutils assumption about a target node and leads not errors. Therefore, do only create a target node if we have ids to work with.
An index directive creates a index node followed by a target node. Two
consecutive index directives::
.. index:: first
.. index:: second
create index, target, index, target, i.e. a mixture. The interspersed
index nodes prevent other transformations, e.g. PropagateTargets to
properly work on the target nodes.
Apply a transformation which reorders mixed and consecutive index and
target nodes such that first all index nodes are before all target
nodes:
Given the following document as input:
<document>
<target ids="id1" ...>
<index entries="...1...">
<target ids="id2" ...>
<target ids="id3" ...>
<index entries="...2...">
<target ids="id4" ...>
The transformed result will be:
<document>
<index entries="...1...">
<index entries="...2...">
<target ids="id1" ...>
<target ids="id2" ...>
<target ids="id3" ...>
<target ids="id4" ...>
latosha-maltba
force-pushed
the
object_description_run_5.x
branch
from
e23f883
to
fb0ad6c
Compare
Thanks for all the feedback! @AA-Turner Once this is merged #9671 can be closed as this is an alternative attempt/proof-of-concept to solving this. |
|
Is there anything that prevents this from being merged or what else can I do to facilitate the adoption of this? |
jakobandersen
approved these changes
Jul 13, 2022
|
@AA-Turner, @tk0miya anything preventing this from being merged? |
# Conflicts: # doc/usage/restructuredtext/domains.rst # sphinx/directives/__init__.py # sphinx/domains/c.py # sphinx/domains/cpp.py # sphinx/domains/javascript.py # sphinx/domains/python.py # sphinx/transforms/__init__.py
AA-Turner
approved these changes
Jul 28, 2023
AA-Turner
added a commit
to AA-Turner/sphinx
that referenced
this pull request
Aug 26, 2023
This resolves a regression introduced in sphinx-docGH-10478 (commit 97d2c5d).
AA-Turner
added a commit
that referenced
this pull request
Aug 26, 2023
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Subject: Only create a target for object descriptions
Feature or Bugfix
Purpose
Adding feature for #9662 (ability to only create a target for a domain description).
Thanks again to @jakobandersen for the inspiration.
This is #9675 repurposed and rebased onto the current 5.x. In the hope that it now gets more traction.
Open Tasks:
:hidden:,:targetonly:,:your-favorite-word-here:, ...Relates