8000 Support redirects from non-padded PEP numbers by AA-Turner · Pull Request #2421 · python/peps · GitHub
[go: up one dir, main page]

Skip to content

Support redirects from non-padded PEP numbers #2421

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

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
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
Prev Previous commit
Next Next commit
Add http-equiv method
  • Loading branch information
AA-Turner committed Mar 12, 2022
commit dd4db10d1e87f2d8840bb6610443740266eb4e03
2 changes: 2 additions & 0 deletions pep_sphinx_extensions/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

from pep_sphinx_extensions.pep_processor.html import pep_html_builder
from pep_sphinx_extensions.pep_processor.html import pep_html_translator
from pep_sphinx_extensions.pep_processor.html.pep_html_redirects import prepare_redirect_pages
from pep_sphinx_extensions.pep_processor.parsing import pep_parser
from pep_sphinx_extensions.pep_processor.parsing import pep_role
from pep_sphinx_extensions.pep_processor.transforms import pep_references
Expand Down Expand Up @@ -64,6 +65,7 @@ def setup(app: Sphinx) -> dict[str, bool]:
# Register event callbacks
app.connect("builder-inited", _update_config_for_builder) # Update configuration values for builder used
app.connect("env-before-read-docs", create_pep_zero) # PEP 0 hook
app.connect("html-collect-pages", prepare_redirect_pages) # Create redirects

# Mathematics rendering
inline_maths = HTMLTranslator.visit_math, _depart_maths
Expand Down
19 changes: 19 additions & 0 deletions pep_sphinx_extensions/pep_processor/html/pep_html_redirects.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
from __future__ import annotations

from typing import TYPE_CHECKING

from pathlib import Path

if TYPE_CHECKING:
from collections.abc import Iterator

from sphinx.application import Sphinx


def prepare_redirect_pages(app: Sphinx) -> Iterator[tuple[str, dict, str]]:
for path in Path(app.srcdir).glob("pep-????.???"):
Copy link
Member
@CAM-Gerlach CAM-Gerlach Mar 13, 2022

Choose a reason for hiding this comment

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

Suggested change
for path in Path(app.srcdir).glob("pep-????.???"):
for path in Path(app.srcdir).glob("pep-[0-9][0-9][0-9][0-9].???"):

Use the correct glob to ensure this is actually a PEP with a valid number. Otherwise, it can match any arbitrary txt or rst file that starts with pep with a four-character suffix, which will cause an error on line 18 (we don't have any right now, but it will suddenly cause a build failure if we ever do, so we may as well just do this properly now.

Copy link
Member Author

Choose a reason for hiding this comment

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

if we ever do

This is I suppose my reluctance to put [0-9] rather than ? -- I seriously think this is very unlikely to happen, and [0-9] is five times as many characters as ?. (The thought applies in all cases that this has been suggested).

A

Copy link
Member

Choose a reason for hiding this comment

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

I hope you didn't stay up all night...or just up really early?

A few more characters perhaps, but in terms of what actually matters, does that make it that much less clear to the reader? And is it really worth sacrificing precision and correctness, and risking a potential future hard build failure just to skimp on a few characters? I realize there is some tradeoff to be made, but it seems the rest of us who have weighed in so far have considered it a worthwhile one.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done. I still disagree though!

A

if path.suffix not in {".txt", ".rst"}:
continue

pep_num = int(path.stem[4:])
yield str(pep_num), {"pep_file": path.stem, "pep_num": pep_num}, "redirect.html"
16 changes: 16 additions & 0 deletions pep_sphinx_extensions/pep_theme/templates/redirect.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
{# Redirect template #}
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="utf-8" />
<meta http-equiv="refresh" content="0; url={{ pathto(pep_file) }}">
<meta name="viewport" content="width=device-width, initial-scale=1.0" />
<title>PEP Redirector | peps.python.org</title>
<link rel="shortcut icon" href="{{ pathto('_static/py.png', resource=True) }}"/>
<link rel="canonical" href="{{ pathto(pep_file) }}" />
<meta name="description" content="Python Enhancement Proposals (PEPs)"/>
</head>
<body>
<p><a href="{{ pathto(pep_file) }}">PEP {{ pep_num }}</a></p>
</body>
</html>
0