fix(release): stop rejecting the changelog entry pkgmgr itself produced
A heading in the message became a bold line, and markdown-lint reads a bold line on its own as a heading: MD036. Every structured entry therefore failed the check it had just been normalised for, and since the loop reopens the editor on failure there was no text that could end it. Headings now move below the "## [version]" line the entry is filed under. A "###" under a "##" is the hierarchy, not a competitor; only "#" and "##" would have collided, and those are the ones raised. The findings never reached the person either. They were printed and the editor took the terminal over in the same breath, so the reason was gone before it could be read. They are held on screen until Enter, and written into the editor buffer as ';' lines above the rejected text, where the fix is actually made; the buffer already drops those lines when it is read back. transform_changelog_message and lint_changelog_entry were each tested alone, which is how a transform that produces lint errors went unnoticed. A test now runs the output of the first through the second. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -7,10 +7,11 @@ import shutil
|
|||||||
import subprocess
|
import subprocess
|
||||||
import tempfile
|
import tempfile
|
||||||
|
|
||||||
_HEADING = re.compile(r"^\s*#{1,6}\s+(.*?)\s*#*\s*$")
|
_HEADING = re.compile(r"^\s*(#{1,6})\s+(.*?)\s*#*\s*$")
|
||||||
_INLINE_CODE = re.compile(r"`([^`\n]+)`")
|
_INLINE_CODE = re.compile(r"`([^`\n]+)`")
|
||||||
_FINDING = re.compile(r"\bMD\d{3}\b")
|
_FINDING = re.compile(r"\bMD\d{3}\b")
|
||||||
_MULTI_BLANK = re.compile(r"\n{3,}")
|
_MULTI_BLANK = re.compile(r"\n{3,}")
|
||||||
|
_ENTRY_LEVEL = 3
|
||||||
|
|
||||||
|
|
||||||
class ChangelogLintError(RuntimeError):
|
class ChangelogLintError(RuntimeError):
|
||||||
@@ -20,10 +21,10 @@ class ChangelogLintError(RuntimeError):
|
|||||||
def transform_changelog_message(text: str) -> str:
|
def transform_changelog_message(text: str) -> str:
|
||||||
"""Normalise a free-form release message into the changelog house style.
|
"""Normalise a free-form release message into the changelog house style.
|
||||||
|
|
||||||
A leading ``#`` heading becomes a bold line of its own (markdown
|
Headings are pushed below the ``## [version]`` line the entry is filed
|
||||||
headings inside an entry body would collide with the ``## [version]``
|
under, so they nest instead of competing with it. Bolding them instead
|
||||||
structure), and inline ``code`` spans become ``*italic*`` so the entry
|
would read as a heading to markdown-lint and fail MD036. Inline ``code``
|
||||||
stays free of backticks.
|
spans become ``*italic*`` so the entry stays free of backticks.
|
||||||
"""
|
"""
|
||||||
text = _INLINE_CODE.sub(r"*\1*", text)
|
text = _INLINE_CODE.sub(r"*\1*", text)
|
||||||
|
|
||||||
@@ -31,9 +32,10 @@ def transform_changelog_message(text: str) -> str:
|
|||||||
for line in text.split("\n"):
|
for line in text.split("\n"):
|
||||||
heading = _HEADING.match(line)
|
heading = _HEADING.match(line)
|
||||||
if heading:
|
if heading:
|
||||||
|
level = max(len(heading.group(1)), _ENTRY_LEVEL)
|
||||||
if out and out[-1].strip():
|
if out and out[-1].strip():
|
||||||
out.append("")
|
out.append("")
|
||||||
out.append(f"**{heading.group(1).strip()}**")
|
out.append(f"{'#' * level} {heading.group(2).strip()}")
|
||||||
out.append("")
|
out.append("")
|
||||||
else:
|
else:
|
||||||
out.append(line)
|
out.append(line)
|
||||||
|
|||||||
@@ -80,6 +80,12 @@ def update_changelog(
|
|||||||
print(f" - {finding}")
|
print(f" - {finding}")
|
||||||
print()
|
print()
|
||||||
|
|
||||||
|
def _wait_for_reader() -> None:
|
||||||
|
try:
|
||||||
|
input("[INFO] Press Enter to re-open the editor and fix the entry...")
|
||||||
|
except EOFError:
|
||||||
|
print("[INFO] Re-opening the editor so you can fix the entry...")
|
||||||
|
|
||||||
if message is not None:
|
if message is not None:
|
||||||
body, entry = _entry_for(message)
|
body, entry = _entry_for(message)
|
||||||
findings = lint_changelog_entry(changelog_path, entry)
|
findings = lint_changelog_entry(changelog_path, entry)
|
||||||
@@ -92,19 +98,21 @@ def update_changelog(
|
|||||||
body, entry = _entry_for(message or f"Release {new_version}")
|
body, entry = _entry_for(message or f"Release {new_version}")
|
||||||
else:
|
else:
|
||||||
attempt: str | None = None
|
attempt: str | None = None
|
||||||
|
rejected: list[str] | None = None
|
||||||
while True:
|
while True:
|
||||||
print(
|
print(
|
||||||
"\n[INFO] Provide the changelog entry — a leading '#' becomes "
|
"\n[INFO] Provide the changelog entry - a leading '#' becomes "
|
||||||
"bold, `code` becomes italic.\n"
|
"a sub-heading, `code` becomes italic.\n"
|
||||||
)
|
)
|
||||||
raw = _open_editor_for_changelog(attempt)
|
raw = _open_editor_for_changelog(attempt, rejected)
|
||||||
body, entry = _entry_for(raw or f"Release {new_version}")
|
body, entry = _entry_for(raw or f"Release {new_version}")
|
||||||
findings = lint_changelog_entry(changelog_path, entry)
|
findings = lint_changelog_entry(changelog_path, entry)
|
||||||
if not findings:
|
if not findings:
|
||||||
break
|
break
|
||||||
_print_findings(findings)
|
_print_findings(findings)
|
||||||
attempt = body
|
attempt = body
|
||||||
print("[INFO] Re-opening the editor so you can fix the entry...")
|
rejected = findings
|
||||||
|
_wait_for_reader()
|
||||||
|
|
||||||
changelog = ""
|
changelog = ""
|
||||||
if os.path.exists(changelog_path):
|
if os.path.exists(changelog_path):
|
||||||
|
|||||||
@@ -6,7 +6,14 @@ import subprocess
|
|||||||
import tempfile
|
import tempfile
|
||||||
|
|
||||||
|
|
||||||
def _open_editor_for_changelog(initial_message: str | None = None) -> str:
|
def _open_editor_for_changelog(
|
||||||
|
initial_message: str | None = None,
|
||||||
|
findings: list[str] | None = None,
|
||||||
|
) -> str:
|
||||||
|
"""Args:
|
||||||
|
initial_message: the previous attempt, pre-loaded for editing.
|
||||||
|
findings: why that attempt was rejected, shown above it.
|
||||||
|
"""
|
||||||
editor = os.environ.get("EDITOR", "nano")
|
editor = os.environ.get("EDITOR", "nano")
|
||||||
|
|
||||||
with tempfile.NamedTemporaryFile(
|
with tempfile.NamedTemporaryFile(
|
||||||
@@ -18,9 +25,15 @@ def _open_editor_for_changelog(initial_message: str | None = None) -> str:
|
|||||||
tmp.write(
|
tmp.write(
|
||||||
"; Write the changelog entry for this release.\n"
|
"; Write the changelog entry for this release.\n"
|
||||||
"; Lines starting with ';' are ignored.\n"
|
"; Lines starting with ';' are ignored.\n"
|
||||||
"; A leading '#' becomes bold; `code` becomes italic.\n"
|
"; A leading '#' becomes a sub-heading; `code` becomes italic.\n"
|
||||||
"; Empty result will fall back to a generic message.\n\n"
|
"; Empty result will fall back to a generic message.\n"
|
||||||
)
|
)
|
||||||
|
if findings:
|
||||||
|
tmp.write(";\n; markdown-lint rejected the previous entry:\n")
|
||||||
|
for finding in findings:
|
||||||
|
for line in str(finding).splitlines() or [""]:
|
||||||
|
tmp.write(f"; {line}\n")
|
||||||
|
tmp.write("\n")
|
||||||
if initial_message:
|
if initial_message:
|
||||||
tmp.write(initial_message.strip() + "\n")
|
tmp.write(initial_message.strip() + "\n")
|
||||||
tmp.flush()
|
tmp.flush()
|
||||||
|
|||||||
@@ -9,15 +9,16 @@ from pkgmgr.actions.release.files.changelog_lint import (
|
|||||||
|
|
||||||
|
|
||||||
class TestTransformChangelogMessage(unittest.TestCase):
|
class TestTransformChangelogMessage(unittest.TestCase):
|
||||||
def test_heading_becomes_bold(self) -> None:
|
def test_heading_is_pushed_below_the_release_heading(self) -> None:
|
||||||
out = transform_changelog_message("# Title\n\nbody")
|
out = transform_changelog_message("# Title\n\nbody")
|
||||||
self.assertIn("**Title**", out)
|
self.assertIn("### Title", out)
|
||||||
self.assertNotIn("# Title", out)
|
self.assertNotIn("**Title**", out)
|
||||||
|
|
||||||
def test_multi_hash_heading_becomes_bold(self) -> None:
|
def test_a_heading_already_deep_enough_keeps_its_level(self) -> None:
|
||||||
self.assertEqual(
|
self.assertEqual(transform_changelog_message("### Sub heading"), "### Sub heading")
|
||||||
transform_changelog_message("### Sub heading"), "**Sub heading**"
|
|
||||||
)
|
def test_a_deeper_heading_is_left_alone(self) -> None:
|
||||||
|
self.assertEqual(transform_changelog_message("#### Deeper"), "#### Deeper")
|
||||||
|
|
||||||
def test_inline_code_becomes_italic(self) -> None:
|
def test_inline_code_becomes_italic(self) -> None:
|
||||||
out = transform_changelog_message("use `pkgmgr release` now")
|
out = transform_changelog_message("use `pkgmgr release` now")
|
||||||
@@ -27,11 +28,12 @@ class TestTransformChangelogMessage(unittest.TestCase):
|
|||||||
def test_plain_message_is_unchanged(self) -> None:
|
def test_plain_message_is_unchanged(self) -> None:
|
||||||
self.assertEqual(transform_changelog_message("just text"), "just text")
|
self.assertEqual(transform_changelog_message("just text"), "just text")
|
||||||
|
|
||||||
def test_no_heading_or_backtick_survives(self) -> None:
|
def test_no_backtick_survives_and_no_heading_competes(self) -> None:
|
||||||
out = transform_changelog_message("# Heading\n\n* item with `code`")
|
out = transform_changelog_message("# Heading\n\n* item with `code`")
|
||||||
self.assertNotIn("`", out)
|
self.assertNotIn("`", out)
|
||||||
for line in out.split("\n"):
|
for line in out.split("\n"):
|
||||||
self.assertFalse(line.lstrip().startswith("#"))
|
if line.lstrip().startswith("#"):
|
||||||
|
self.assertTrue(line.startswith("###"), line)
|
||||||
|
|
||||||
|
|
||||||
class TestLintChangelogEntry(unittest.TestCase):
|
class TestLintChangelogEntry(unittest.TestCase):
|
||||||
@@ -39,6 +41,13 @@ class TestLintChangelogEntry(unittest.TestCase):
|
|||||||
entry = "## [1.0.0] - 2026-01-01\n\n* a clean bullet\n\n"
|
entry = "## [1.0.0] - 2026-01-01\n\n* a clean bullet\n\n"
|
||||||
self.assertEqual(lint_changelog_entry("CHANGELOG.md", entry), [])
|
self.assertEqual(lint_changelog_entry("CHANGELOG.md", entry), [])
|
||||||
|
|
||||||
|
def test_a_structured_message_survives_its_own_transform(self) -> None:
|
||||||
|
body = transform_changelog_message(
|
||||||
|
"# Security\n\n* a bullet\n\n## Fixed\n\n* another bullet"
|
||||||
|
)
|
||||||
|
entry = f"## [1.0.0] - 2026-01-01\n\n{body}\n\n"
|
||||||
|
self.assertEqual(lint_changelog_entry("CHANGELOG.md", entry), [])
|
||||||
|
|
||||||
|
|
||||||
if __name__ == "__main__": # pragma: no cover
|
if __name__ == "__main__": # pragma: no cover
|
||||||
unittest.main()
|
unittest.main()
|
||||||
|
|||||||
60
tests/unit/pkgmgr/actions/release/test_editor.py
Normal file
60
tests/unit/pkgmgr/actions/release/test_editor.py
Normal file
@@ -0,0 +1,60 @@
|
|||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import unittest
|
||||||
|
from unittest import mock
|
||||||
|
|
||||||
|
from pkgmgr.actions.release.files.editor import _open_editor_for_changelog
|
||||||
|
|
||||||
|
|
||||||
|
class TestOpenEditorForChangelog(unittest.TestCase):
|
||||||
|
def _buffer(self, **kwargs: object) -> str:
|
||||||
|
"""Returns: what the editor was handed, with the editor itself a no-op."""
|
||||||
|
seen: dict[str, str] = {}
|
||||||
|
|
||||||
|
def record(argv: list[str]) -> int:
|
||||||
|
with open(argv[1], encoding="utf-8") as handle:
|
||||||
|
seen["text"] = handle.read()
|
||||||
|
return 0
|
||||||
|
|
||||||
|
with mock.patch(
|
||||||
|
"pkgmgr.actions.release.files.editor.subprocess.call", side_effect=record
|
||||||
|
):
|
||||||
|
_open_editor_for_changelog(**kwargs)
|
||||||
|
return seen["text"]
|
||||||
|
|
||||||
|
def test_the_rejection_reaches_the_editor_the_entry_is_fixed_in(self) -> None:
|
||||||
|
text = self._buffer(
|
||||||
|
initial_message="### Security\n\n- a bullet",
|
||||||
|
findings=["changelog entry:3 error MD036/no-emphasis-as-heading"],
|
||||||
|
)
|
||||||
|
self.assertIn("MD036/no-emphasis-as-heading", text)
|
||||||
|
self.assertIn("### Security", text)
|
||||||
|
|
||||||
|
def test_every_finding_line_is_commented_out(self) -> None:
|
||||||
|
text = self._buffer(findings=["first line\nsecond line"])
|
||||||
|
for line in text.splitlines():
|
||||||
|
if "line" in line and "ignored" not in line:
|
||||||
|
self.assertTrue(line.startswith(";"), line)
|
||||||
|
|
||||||
|
def test_a_finding_never_lands_in_the_entry(self) -> None:
|
||||||
|
seen: dict[str, str] = {}
|
||||||
|
|
||||||
|
def rewrite(argv: list[str]) -> int:
|
||||||
|
with open(argv[1], encoding="utf-8") as handle:
|
||||||
|
seen["text"] = handle.read()
|
||||||
|
return 0
|
||||||
|
|
||||||
|
with mock.patch(
|
||||||
|
"pkgmgr.actions.release.files.editor.subprocess.call", side_effect=rewrite
|
||||||
|
):
|
||||||
|
kept = _open_editor_for_changelog(
|
||||||
|
initial_message="- a bullet", findings=["MD036 somewhere"]
|
||||||
|
)
|
||||||
|
self.assertEqual(kept, "- a bullet")
|
||||||
|
|
||||||
|
def test_without_findings_the_header_stays_as_it_was(self) -> None:
|
||||||
|
self.assertNotIn("markdown-lint rejected", self._buffer())
|
||||||
|
|
||||||
|
|
||||||
|
if __name__ == "__main__": # pragma: no cover
|
||||||
|
unittest.main()
|
||||||
@@ -334,9 +334,10 @@ class TestUpdateChangelog(unittest.TestCase):
|
|||||||
with open(path, encoding="utf-8") as f:
|
with open(path, encoding="utf-8") as f:
|
||||||
content = f.read()
|
content = f.read()
|
||||||
|
|
||||||
self.assertIn("**Summary**", content)
|
self.assertIn("### Summary", content)
|
||||||
self.assertIn("*foo*", content)
|
self.assertIn("*foo*", content)
|
||||||
self.assertNotIn("# Summary", content)
|
self.assertNotIn("\n# Summary", content)
|
||||||
|
self.assertNotIn("\n## Summary", content)
|
||||||
self.assertNotIn("`foo`", content)
|
self.assertNotIn("`foo`", content)
|
||||||
|
|
||||||
def test_update_changelog_preview_does_not_write(self) -> None:
|
def test_update_changelog_preview_does_not_write(self) -> None:
|
||||||
|
|||||||
Reference in New Issue
Block a user