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.

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

Fix non-unique heading anchors on changelog doc page #1942

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ repos:
- toml==0.10.2
- pip==20.3.4
- build==0.9.0
- types-docutils==0.20.0.1
- repo: https://github.com/PyCQA/bandit
rev: 1.7.5
hooks:
Expand Down
Empty file added docs/__init__.py
Copy link
Member

Choose a reason for hiding this comment

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

Don't add Python modules among the normal documents.

Empty file.
13 changes: 13 additions & 0 deletions docs/conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,19 @@

from __future__ import annotations

import os
import sys
from importlib.metadata import version as get_version
from pathlib import Path

from sphinx.application import Sphinx
Copy link
Member

Choose a reason for hiding this comment

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

Obviously, this won't be necessary upon moving to a proper extension.

from sphinx.util import logging
from sphinx.util.console import bold

# Add the docs/ directory to sys.path to be able to import utils
docs_dir = os.path.dirname(os.path.dirname(__file__))
Copy link
Member

Choose a reason for hiding this comment

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

Plz use pathlib and only convert to str on insert.

sys.path.insert(0, docs_dir)

logger = logging.getLogger(__name__)

# -- Path setup --------------------------------------------------------------
Expand Down Expand Up @@ -57,3 +64,9 @@
]

suppress_warnings = ["myst.xref_missing"]


def setup(app: Sphinx) -> None:
Copy link
Member

Choose a reason for hiding this comment

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

Don't add setup() here but put it into an extension under _ext/.

Also, it should return a dict.

from docs.utils import TransformSectionIdToName
Copy link
Member

Choose a reason for hiding this comment

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

This is not needed, especially upon moving to the extension. Just add the extension to the list above and Sphinx will load it through the standard means.


app.add_transform(TransformSectionIdToName)
34 changes: 34 additions & 0 deletions docs/utils.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
from __future__ import annotations
Copy link
Member

Choose a reason for hiding this comment

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

Move this to the _ext/ subir per convention.


import re
from typing import Any

from docutils import nodes
from sphinx.transforms import SortIds, SphinxTransform

numerical_id = re.compile(r"^id[0-9]+$")


class TransformSectionIdToName(SphinxTransform): # type: ignore[misc]
Copy link
Member

Choose a reason for hiding this comment

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

What does this ignore?

"""Transforms section ids from <id1>, <id2>, ... to id <section-name>."""

default_priority = SortIds.default_priority + 1

def apply(self, **kwargs: Any) -> Any:
Copy link
Member

Choose a reason for hiding this comment

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

This returns none, not any.

for node in self.document.findall(nodes.section):
if not self._has_numerical_id(node):
continue
self._transform_id_to_name(node)

def _has_numerical_id(self, node: nodes.section) -> bool:
node_ids = node["ids"]
if not node_ids:
return False

if len(node_ids) != 1:
return False

return bool(numerical_id.match(node_ids[0]))
Comment on lines +25 to +31
Copy link
Member

Choose a reason for hiding this comment

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

This can be simplified:

Suggested change
if not node_ids:
return False
if len(node_ids) != 1:
return False
return bool(numerical_id.match(node_ids[0]))
node_has_single_id = len(node_ids) == 1
return node_has_single_id and
numerical_id.match(node_ids[0]) is not None


def _transform_id_to_name(self, node: nodes.section) -> None:
node["ids"] = [nodes.make_id("id " + name) for name in node["names"]]
Copy link
Member

Choose a reason for hiding this comment

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

String concatenation is slow.

Suggested change
node["ids"] = [nodes.make_id("id " + name) for name in node["names"]]
node["ids"] = [nodes.make_id(f"id {node_name}") for node_name in node["names"]]