Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add faster TocTree._toctree_copy method #10988

Merged
merged 4 commits into from Jan 2, 2023
Merged

Add faster TocTree._toctree_copy method #10988

Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
07d965c
Speed up navigation generation for "-M html" builder
hofmandl1 Nov 23, 2022
2bfbfe7
Merge branch 'master' into master
AA-Turner Dec 29, 2022
120654c
Merge branch 'master' into toctree-copy
AA-Turner Jan 2, 2023
d0904a9
Remove ``_toctree_prune``; factor out booleans
AA-Turner Jan 2, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
45 changes: 39 additions & 6 deletions sphinx/environment/adapters/toctree.py
View file
Open in desktop
@@ -1,6 +1,6 @@
"""Toctree adapter for sphinx.environment."""

from typing import TYPE_CHECKING, Any, Dict, Iterable, List, Optional, Tuple, cast
from typing import TYPE_CHECKING, Any, Dict, Iterable, List, Optional, Tuple, TypeVar, cast

from docutils import nodes
from docutils.nodes import Element, Node
Expand Down Expand Up @@ -158,10 +158,12 @@ def _entries_from_toctree(toctreenode: addnodes.toctree, parents: List[str],
location=ref, type='toc', subtype='circular')
continue
refdoc = ref
toc = self.env.tocs[ref].deepcopy()
maxdepth = self.env.metadata[ref].get('tocdepth', 0)
toc = self.env.tocs[ref]
if ref not in toctree_ancestors or (prune and maxdepth > 0):
self._toctree_prune(toc, 2, maxdepth, collapse)
toc = self._toctree_copy(toc, 2, maxdepth, collapse)
else:
toc = toc.deepcopy()
process_only_nodes(toc, builder.tags)
if title and toc.children and len(toc.children) == 1:
child = toc.children[0]
Expand Down Expand Up @@ -281,9 +283,41 @@ def get_toctree_ancestors(self, docname: str) -> List[str]:
d = parent[d]
return ancestors

ET = TypeVar('ET', bound=Element)

def _toctree_copy(self, node: ET, depth: int, maxdepth: int, collapse: bool = False) -> ET:
"""Utility: deep-copy a TOC but omit things with the semantics of ._toctree_prune.

!!! MUST be kept consistent with ._toctree_prune !!!
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you mean by 'consistent' here?

A

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While _toctree_prune deletes unneeded stuff from a (previously done) deep copy of the toctree, _toctree_copy does the deep copy itself but only copies the parts, that _toctree_prune would not delete. So assuming, that _toctree_prune would be adjusted in order to e.g. not delete a particular node type, then _toctree_copy would also need to be adjusted to copy that particular node type.

TBH we didn't fully understand the different node types involved here. It would probably be best, to completely get rid of _toctree_prune in order not to have this consistency problem any more (maybe it is not that big of a problem if there aren't any adjustments to that code done in the foreseeable future). We tried getting rid of _toctree_prune but found that it is used in another place and didn't feel confident enough to do the necessary changes.

"""
copy = node.copy()
for subnode in node.children:
if isinstance(subnode, (addnodes.compact_paragraph,
nodes.list_item)):
# for <p> and <li>, just recurse
copy.append(self._toctree_copy(subnode, depth, maxdepth, collapse))
elif isinstance(subnode, nodes.bullet_list):
# for <ul>, determine if the depth is too large or if the
# entry is to be collapsed
if maxdepth > 0 and depth > maxdepth:
pass
else:
# cull sub-entries whose parents aren't 'current'
if (collapse and depth > 1 and
'iscurrent' not in subnode.parent):
pass
else:
# recurse on visible children
copy.append(self._toctree_copy(subnode, depth + 1, maxdepth, collapse))
else:
copy.append(subnode.deepcopy())
return copy

def _toctree_prune(self, node: Element, depth: int, maxdepth: int, collapse: bool = False
) -> None:
"""Utility: Cut a TOC at a specified depth."""
"""Utility: Cut a TOC at a specified depth.

!!! MUST be kept consistent with ._toctree_copy !!!"""
for subnode in node.children[:]:
if isinstance(subnode, (addnodes.compact_paragraph,
nodes.list_item)):
Expand All @@ -307,8 +341,7 @@ def get_toc_for(self, docname: str, builder: "Builder") -> Node:
"""Return a TOC nodetree -- for use on the same page only!"""
tocdepth = self.env.metadata[docname].get('tocdepth', 0)
try:
toc = self.env.tocs[docname].deepcopy()
self._toctree_prune(toc, 2, tocdepth)
toc = self._toctree_copy(self.env.tocs[docname], 2, tocdepth)
except KeyError:
# the document does not exist anymore: return a dummy node that
# renders to nothing
Expand Down