From a4feace6254e9609fd4a5d3ecb93491fd6f33ce8 Mon Sep 17 00:00:00 2001 From: Jelle Zijlstra Date: Wed, 16 Oct 2024 06:36:48 -0700 Subject: [PATCH 1/6] Update pre-commit hook --- .pre-commit-config.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index da6e98e9755..d90cf896050 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -211,9 +211,9 @@ repos: files: '^peps/pep-\d+\.rst$' - id: validate-resolution - name: "'Resolution' must be a direct thread/message URL" + name: "'Resolution' must be a direct thread/message URL or '`DD-mmm-YYYY `__'" language: pygrep - entry: '(?`__)' args: ['--multiline'] files: '^peps/pep-\d+\.rst$' From d764a9d126d2d0c38fa18012c0d066f6a490b969 Mon Sep 17 00:00:00 2001 From: Jelle Zijlstra Date: Wed, 16 Oct 2024 06:42:11 -0700 Subject: [PATCH 2/6] Allow resolution date in Resolution header Fixes #4054 --- .pre-commit-config.yaml | 2 +- check-peps.py | 32 ++++++++++++++++--- .../pep_processor/transforms/pep_headers.py | 4 +++ .../tests/pep_lint/test_post_url.py | 1 + peps/pep-0001.rst | 2 +- peps/pep-0702.rst | 2 +- peps/pep-0735.rst | 2 +- 7 files changed, 36 insertions(+), 9 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index d90cf896050..b36a5b105c4 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -213,7 +213,7 @@ repos: - id: validate-resolution name: "'Resolution' must be a direct thread/message URL or '`DD-mmm-YYYY `__'" language: pygrep - entry: '(?`__)' + entry: '(?`__))\n' args: ['--multiline'] files: '^peps/pep-\d+\.rst$' diff --git a/check-peps.py b/check-peps.py index 93d367379cf..cf167f8ac6c 100755 --- a/check-peps.py +++ b/check-peps.py @@ -407,7 +407,7 @@ def _validate_python_version(line_num: int, line: str) -> MessageIterator: def _validate_post_history(line_num: int, body: str) -> MessageIterator: - """'Post-History' must be '`DD-mmm-YYYY `__, …'""" + """'Post-History' must be '`DD-mmm-YYYY `__, …' or `DD-mmm-YYYY`""" if body == "": return @@ -422,19 +422,41 @@ def _validate_post_history(line_num: int, body: str) -> MessageIterator: yield from _date(offset, post_date, "Post-History") yield from _thread(offset, post_url, "Post-History") else: - yield offset, f"post line must be a date or both start with “`” and end with “>`__”" + yield offset, "post line must be a date or both start with “`” and end with “>`__”" def _validate_resolution(line_num: int, line: str) -> MessageIterator: - """'Resolution' must be a direct thread/message URL""" - - yield from _thread(line_num, line, "Resolution", allow_message=True) + """'Resolution' must be a direct thread/message URL or a link with a date.""" + + prefix, postfix = (line.startswith("`"), line.endswith(">`__")) + if not prefix and not postfix: + yield from _thread(line_num, line, "Resolution", allow_message=True) + elif prefix and postfix: + post_date, post_url = line[1:-4].split(" <") + yield from _date(line_num, post_date, "Resolution") + yield from _thread(line_num, post_url, "Resolution") + else: + yield line_num, "Resolution line must be a link or both start with “`” and end with “>`__”" ######################## # Validation Helpers # ######################## +def _validate_link_or_thread(text: str, header: str, offset: int, *, allow_message: bool = False) -> MessageIterator: + prefix, postfix = (text.startswith("`"), text.endswith(">`__")) + if not prefix and not postfix: + if header == "Resolution": + yield from _thread(line_num, line, "Resolution", allow_message=True) + yield from _date(offset, text, header) + elif prefix and postfix: + post_date, post_url = text[1:-4].split(" <") + yield from _date(offset, post_date, header) + yield from _thread(offset, post_url, header) + else: + yield offset, f"{header} line must be a date or both start with “`” and end with “>`__”" + + def _pep_num(line_num: int, pep_number: str, prefix: str) -> MessageIterator: if pep_number == "": yield line_num, f"{prefix} must not be blank: {pep_number!r}" diff --git a/pep_sphinx_extensions/pep_processor/transforms/pep_headers.py b/pep_sphinx_extensions/pep_processor/transforms/pep_headers.py index a7cd0c3030a..f95b054b51a 100644 --- a/pep_sphinx_extensions/pep_processor/transforms/pep_headers.py +++ b/pep_sphinx_extensions/pep_processor/transforms/pep_headers.py @@ -116,9 +116,13 @@ def apply(self) -> None: elif name in {"discussions-to", "resolution", "post-history"}: # Prettify mailing list and Discourse links for node in para: + print(name, para, node) if (not isinstance(node, nodes.reference) or not node["refuri"]): continue + # If the Resolution header is already a link, don't prettify it + if name == "resolution" and node["refuri"] != node[0]: + continue # Have known mailto links link to their main list pages if node["refuri"].lower().startswith("mailto:"): node["refuri"] = _generate_list_url(node["refuri"]) diff --git a/pep_sphinx_extensions/tests/pep_lint/test_post_url.py b/pep_sphinx_extensions/tests/pep_lint/test_post_url.py index 982f4612eea..b2e54725705 100644 --- a/pep_sphinx_extensions/tests/pep_lint/test_post_url.py +++ b/pep_sphinx_extensions/tests/pep_lint/test_post_url.py @@ -90,6 +90,7 @@ def test_validate_post_history_valid(body: str): "https://mail.python.org/archives/list/list-name@python.org/message/abcXYZ123/#Anchor", "https://mail.python.org/archives/list/list-name@python.org/message/abcXYZ123#Anchor123", "https://mail.python.org/archives/list/list-name@python.org/message/abcXYZ123/#Anchor123", + "`16-Oct-2024 `__", ], ) def test_validate_resolution_valid(line: str): diff --git a/peps/pep-0001.rst b/peps/pep-0001.rst index 28c33f8d9af..0b257ff6690 100644 --- a/peps/pep-0001.rst +++ b/peps/pep-0001.rst @@ -621,7 +621,7 @@ optional and are described below. All other headers are required. inline-linked to PEP discussion threads> * Replaces: * Superseded-By: - * Resolution: + * Resolution: The Author header lists the names, and optionally the email addresses of all the authors/owners of the PEP. The format of the Author header diff --git a/peps/pep-0702.rst b/peps/pep-0702.rst index b7f2f30646e..241b5bf31e2 100644 --- a/peps/pep-0702.rst +++ b/peps/pep-0702.rst @@ -9,7 +9,7 @@ Created: 30-Dec-2022 Python-Version: 3.13 Post-History: `01-Jan-2023 `__, `22-Jan-2023 `__ -Resolution: https://discuss.python.org/t/pep-702-marking-deprecations-using-the-type-system/23036/61 +Resolution: `07-Nov-2023 `__ .. canonical-typing-spec:: :ref:`typing:deprecated` and :external+py3.13:func:`@warnings.deprecated` diff --git a/peps/pep-0735.rst b/peps/pep-0735.rst index 455bc7e53d0..0ccbfcd133b 100644 --- a/peps/pep-0735.rst +++ b/peps/pep-0735.rst @@ -9,7 +9,7 @@ Type: Standards Track Topic: Packaging Created: 20-Nov-2023 Post-History: `14-Nov-2023 `__, `20-Nov-2023 `__ -Resolution: https://discuss.python.org/t/39233/312 +Resolution: `10-Oct-2024 `__ Abstract From 5b11bd699748410533d8d7c3a8f1618f876ad0b8 Mon Sep 17 00:00:00 2001 From: Jelle Zijlstra Date: Wed, 16 Oct 2024 06:43:43 -0700 Subject: [PATCH 3/6] remove pinrt --- pep_sphinx_extensions/pep_processor/transforms/pep_headers.py | 1 - 1 file changed, 1 deletion(-) diff --git a/pep_sphinx_extensions/pep_processor/transforms/pep_headers.py b/pep_sphinx_extensions/pep_processor/transforms/pep_headers.py index f95b054b51a..34ae7cfc623 100644 --- a/pep_sphinx_extensions/pep_processor/transforms/pep_headers.py +++ b/pep_sphinx_extensions/pep_processor/transforms/pep_headers.py @@ -116,7 +116,6 @@ def apply(self) -> None: elif name in {"discussions-to", "resolution", "post-history"}: # Prettify mailing list and Discourse links for node in para: - print(name, para, node) if (not isinstance(node, nodes.reference) or not node["refuri"]): continue From 45f236566aae0b429e0fe52e3fc339c768a083a7 Mon Sep 17 00:00:00 2001 From: Jelle Zijlstra Date: Wed, 16 Oct 2024 06:45:56 -0700 Subject: [PATCH 4/6] Allow message --- check-peps.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/check-peps.py b/check-peps.py index cf167f8ac6c..f740788a727 100755 --- a/check-peps.py +++ b/check-peps.py @@ -434,7 +434,7 @@ def _validate_resolution(line_num: int, line: str) -> MessageIterator: elif prefix and postfix: post_date, post_url = line[1:-4].split(" <") yield from _date(line_num, post_date, "Resolution") - yield from _thread(line_num, post_url, "Resolution") + yield from _thread(line_num, post_url, "Resolution", allow_message=True) else: yield line_num, "Resolution line must be a link or both start with “`” and end with “>`__”" From 1e6029f5577ad19d016e79917ac09e87384b9383 Mon Sep 17 00:00:00 2001 From: Jelle Zijlstra Date: Wed, 16 Oct 2024 07:19:52 -0700 Subject: [PATCH 5/6] remove stray function --- check-peps.py | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/check-peps.py b/check-peps.py index f740788a727..58b842809a3 100755 --- a/check-peps.py +++ b/check-peps.py @@ -443,19 +443,6 @@ def _validate_resolution(line_num: int, line: str) -> MessageIterator: # Validation Helpers # ######################## -def _validate_link_or_thread(text: str, header: str, offset: int, *, allow_message: bool = False) -> MessageIterator: - prefix, postfix = (text.startswith("`"), text.endswith(">`__")) - if not prefix and not postfix: - if header == "Resolution": - yield from _thread(line_num, line, "Resolution", allow_message=True) - yield from _date(offset, text, header) - elif prefix and postfix: - post_date, post_url = text[1:-4].split(" <") - yield from _date(offset, post_date, header) - yield from _thread(offset, post_url, header) - else: - yield offset, f"{header} line must be a date or both start with “`” and end with “>`__”" - def _pep_num(line_num: int, pep_number: str, prefix: str) -> MessageIterator: if pep_number == "": From 1aca0a06f15093ecaeb95b1f3738c8564f029b05 Mon Sep 17 00:00:00 2001 From: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com> Date: Wed, 16 Oct 2024 17:56:33 +0300 Subject: [PATCH 6/6] Add tests to increase coverage --- .../tests/pep_lint/test_pep_lint.py | 9 ++++++ .../tests/pep_lint/test_post_url.py | 28 +++++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/pep_sphinx_extensions/tests/pep_lint/test_pep_lint.py b/pep_sphinx_extensions/tests/pep_lint/test_pep_lint.py index 2c52fa3977e..ab8e4105775 100644 --- a/pep_sphinx_extensions/tests/pep_lint/test_pep_lint.py +++ b/pep_sphinx_extensions/tests/pep_lint/test_pep_lint.py @@ -2,6 +2,8 @@ import check_peps # NoQA: inserted into sys.modules in conftest.py +from ..conftest import PEP_ROOT + PEP_9002 = Path(__file__).parent.parent / "peps" / "pep-9002.rst" @@ -46,3 +48,10 @@ def test_with_fake_pep(): (20, "Resolution must be a valid thread URL"), (23, "Use the :pep:`NNN` role to refer to PEPs"), ] + + +def test_skip_direct_pep_link_check(): + filename = PEP_ROOT / "pep-0009.rst" # in SKIP_DIRECT_PEP_LINK_CHECK + content = filename.read_text(encoding="utf-8").splitlines() + warnings = list(check_peps.check_peps(filename, content)) + assert warnings == [] diff --git a/pep_sphinx_extensions/tests/pep_lint/test_post_url.py b/pep_sphinx_extensions/tests/pep_lint/test_post_url.py index b2e54725705..15d4739a5b1 100644 --- a/pep_sphinx_extensions/tests/pep_lint/test_post_url.py +++ b/pep_sphinx_extensions/tests/pep_lint/test_post_url.py @@ -79,6 +79,20 @@ def test_validate_post_history_valid(body: str): assert warnings == [], warnings +@pytest.mark.parametrize( + "body", + [ + "31-Jul-2015 `__,", + "`31-Jul-2015 ", + ], +) +def test_validate_post_history_unbalanced_link(body: str): + warnings = [warning for (_, warning) in check_peps._validate_post_history(1, body)] + assert warnings == [ + "post line must be a date or both start with “`” and end with “>`__”" + ], warnings + + @pytest.mark.parametrize( "line", [ @@ -118,6 +132,20 @@ def test_validate_resolution_invalid(line: str): assert warnings == ["Resolution must be a valid thread URL"], warnings +@pytest.mark.parametrize( + "line", + [ + "01-Jan-2000 `__", + "`01-Jan-2000 ", + ], +) +def test_validate_resolution_unbalanced_link(line: str): + warnings = [warning for (_, warning) in check_peps._validate_resolution(1, line)] + assert warnings == [ + "Resolution line must be a link or both start with “`” and end with “>`__”" + ], warnings + + @pytest.mark.parametrize( "thread_url", [