docs: create underscored versions of every page as a redirect target#540
Conversation
james-garner-canonical
force-pushed
the
26-06+docs+redirect-underscores
branch
from
1cd5cfe to
8d987c3
Compare
tonyandrewmeyer
left a comment
tonyandrewmeyer
left a comment
There was a problem hiding this comment.
Do you have some examples that would be easy to test this on?
| @@ -0,0 +1,168 @@ | |||
| # Copyright 2025 Canonical Ltd. | |||
There was a problem hiding this comment.
| # Copyright 2025 Canonical Ltd. | |
| # Copyright 2026 Canonical Ltd. |
| @@ -0,0 +1,106 @@ | |||
| # Copyright 2025 Canonical Ltd. | |||
There was a problem hiding this comment.
| # Copyright 2025 Canonical Ltd. | |
| # Copyright 2026 Canonical Ltd. |
| return | ||
| redirects = _build_redirects(set(env.found_docs)) | ||
| target = typing.cast('dict[str, str]', app.config.rediraffe_redirects) | ||
| assert isinstance(target, dict), f'rediraffe_redirects must be a dict, not {target}' |
There was a problem hiding this comment.
Since the complaint is the type, do you think it would be clearer to have the type in the error rather than whatever the str is? Or even just the repr maybe?
| redirects = _build_redirects(set(env.found_docs)) | ||
| target = typing.cast('dict[str, str]', app.config.rediraffe_redirects) | ||
| assert isinstance(target, dict), f'rediraffe_redirects must be a dict, not {target}' | ||
| if not set(target).isdisjoint(redirects.keys()): |
There was a problem hiding this comment.
It seems odd that one of these implicitly uses "iter dict is keys" and the other has it explicit. Is that to satisfy some type check?
| for k, v in redirects.items() | ||
| if k in target | ||
| ) | ||
| raise ValueError('\n - '.join(lines)) |
There was a problem hiding this comment.
I don't love a multi-line message in a ValueError. Where does this show up, in the build output? Do you have an example of what that looks like? If it looks good and is the only place it would be expected, I would be convinced I think. Same for the ones below.
| return {'version': '1.0.0', 'parallel_read_safe': True, 'parallel_write_safe': True} | ||
|
|
||
|
|
||
| def _automatic_redirects( |
There was a problem hiding this comment.
Is this the best name? It feels like most of what it's doing is validation.
| category, | ||
| category.replace('-', '_'), # how_to | ||
| category.replace('_', '-'), # how-to | ||
| category.replace('_', '').replace('-', ''), # howto |
There was a problem hiding this comment.
Does this mean it's not just "how-to" -> "howto", but also all other _ and - removal cases, like "my-first-trace" -> "myfirsttrace"?
I understand the motivation for hyphen/underscore redirects, but I'm less sure about removing them. Is this an issue we see in practice?
| def _separator_variant(docname: str) -> str: | ||
| """Return the docname with ``-`` and ``_`` swapped.""" | ||
| if '-' in docname: | ||
| assert '_' not in docname, f"Docname {docname} should not contain both '-' and '_'" |
There was a problem hiding this comment.
I feel this ought to be a ValueError not an assertion. Also below.
| assert alias not in redirects, ( | ||
| f'Alias {alias} already redirects to {redirects[alias]}' | ||
| ) | ||
| if docname.endswith('/index.html'): |
There was a problem hiding this comment.
Do we need to handle the llms.txt markdown files here too?
2 participants
This PR adds a custom Sphinx extension to automatically create redirects from URLs using underscores to the canonical page using hyphens (and vice versa, and 'howto' -> 'how-to').
Hyphens vs underscores are a common source of confusion in creating working links to library and interface docs (e.g. coming up frequently in PRs). Our contributor docs and PR reviews aim to avoid errors when adding URLs, but this is hard to check automatically (e.g. the docs URL in a PyPI README.md to the docs may not exist on the docs site at time of PR merge), and even if we get that solved, we can't control the links committed outside this repository. Automatically ensuring users end up on the right canonical page would alleviate this issue.
Resolves #541.