fix(changelog-md): cite a line the entry has, and accept an anchored :rfc: role (#588, #592) - #595
Conversation
…:rfc: role (#588, #592) * ResidualMarkupError named the source entry file but numbered its lines against the converted body, so the number it printed could not exist in the file it pointed at. Rule 6 joins each block onto one line: measured on the 1.5.0 entry the body is 55 lines against the file's 580, and four roles written on source lines 467 and 468 were all reported as "line 46". The bound alone settles it -- a number that cannot exceed 55 cannot name a line in a 580-line file. convert_traced() now returns a source map beside the Markdown, recording which line of the entry each stretch of output came from, and residual() resolves every hit through it. That also separates the hits: constructs joined onto one output line become one complaint each, at their own lines, in the entry's order rather than the pattern list's. residual() still takes a bare string, and then labels its numbers "converted line N" instead of passing an unusable number off as a location. * Rule 2 accepted only :rfc:`NNNN`, so :rfc:`NNNN#section-3` -- the spelling Sphinx also accepts, and the one anyone citing a section reaches for -- fell past the rule that exists for it and was rejected as a role the rules do not cover. Both spellings convert now, with the anchor carried into the target. The link text comes from sphinx.roles._format_rfc_target rather than being invented here, so an entry reads the same in CHANGELOG.md as in the rendered history; an anchor of a shape that helper does not name still reaches the guard rather than being carried into a link nobody checked. * Two supporting changes the map needs. Masking a code span is now length- preserving, so a match offset in the masked text is an offset in the Markdown; no pattern counts markers, so nothing about what matches moves. And rule 6 collapses runs of blank lines structurally rather than with a \n{3,} substitution over the joined text, which would have shifted every offset -- same result between blocks, and one better before the first block, where the substitution used to leave an empty line behind. * The `` literal branch of the guard reports one complaint per literal rather than per output line, and had no test of its own until now. Regeneration is unchanged: 36 of the 37 committed entries convert byte for byte as before, and the 37th differs by exactly the one bullet added here. python util/changelog_md.py --check exits 0. tests/project/ green: 64 passed, 469 subtests. The 13 new cases in tests/project/test_changelog_md.py were run against the committed converter first and fail there (46 tests, 9 failures and 2 errors, exit 1) and pass with the fix (47 tests, exit 0); util/changelog_md.py rises from 96% to 98% statement and branch coverage, the remainder being the __main__ guard and a pre-existing break in read_toctree().
a23d8bb to
1719275
Compare
|
✅ GOOD TO MERGE — head |
Cross-review appendix — PR #595Reviewer: Sonnet; PR authored on Opus 5. Reviewed at head
|
1 participant
Fixes #588
Fixes #592
Both defects live in the same error path of
util/changelog_md.py— theChangelog driftgate that blocked two of the five pull requests opened against this repo today. Neither changes what the converter emits; they change what it says when it refuses.#588 — a line number counted in the wrong frame
ResidualMarkupErrornamed the entry file and numbered its lines against the converted body. Re-measured on this tree before fixing anything:Four roles, two source lines, one reported number — and 46 is neither of them. The bound settles it on its own: a number that cannot exceed 55 cannot name a line in a 580-line file.
Fix shape: the source map, not the relabelling. The issue offered both. Relabelling ("line 46 of the converted entry") would have made the message honest but still useless — the author is being told to edit a file, and rule 6 joins ten source lines into one, so the nine offences that blocked #586 would still arrive as nine copies of one number. So
convert_traced()now returns a(offset, line)map beside the Markdown, andresidual()resolves each hit through it. Same input, after:The map is cheap because rules 1–5 rewrite a line in place — one map entry per source line — and only rule 6 joins. Complaints now also come out one per construct (not one per joined output line) and in the entry's order rather than the pattern list's.
residual()still accepts a bare string, and then saysconverted line N, because the relabelling is the right answer when there is genuinely nothing to map through.#592 — rule 2 read only half the role
Widened, as the issue suggested.
:rfc:`6554#section-3was falling past the rule that exists for it and being reported as a role the rules do not cover.The link text is taken from
sphinx.roles._format_rfc_target(read from Sphinx 9.1.0, the version this repo builds docs with) rather than invented here, so an entry reads the same inCHANGELOG.mdas in the rendered history:[RFC 6554 Section 3](https://datatracker.ietf.org/doc/html/rfc6554#section-3), with the three anchor prefixes Sphinx titles (section,appendix,page) and every other anchor left as written. The pattern is deliberately(?:#([\w.-]+))?rather than "anything between the backticks": an anchor of a shape the helper cannot name still reaches the guard rather than being carried into a link nobody checked.This PR's own changelog bullet cites
:rfc:6554#section-3`` and passes the gate, which is the first end-to-end test of the fix.Two supporting changes the map needs
\n{3,}substitution over the joined text, which would have shifted every offset. Same result between blocks; one better before the first block, where the substitution used to leave an empty line behind.Regeneration is unchanged
The obligation this PR is most at risk of breaking, so it is measured rather than asserted.
convert()was snapshotted over all 37 committed entries before the change and compared after:git diff --statagrees:CHANGELOG.md | 1 +.python util/changelog_md.py --checkexits 0.Tests
13 new cases in
tests/project/test_changelog_md.py, run against the committed converter first (same test file,util/changelog_md.pyfrommain) and then against the fix:Ran 46 tests—FAILED (failures=9, errors=2)Ran 47 tests—OKSample of what the before-run says, verbatim:
The location assertions deliberately check the entry file rather than the wording: a cited line is read back out of the entry and has to hold the construct it was cited for.
tests/project/is green (64 passed, 469 subtests). Coverage ofutil/changelog_md.pyrises 96% → 98% statement and branch (coverage run --source=util --include='util/changelog_md.py' -m pytest tests/project/); what remains is the__main__guard and a pre-existingbreakinread_toctree(). TheFound, not fixed
]+)`` `` pairs the nearest backticks it can, soana`band ac`dliteral` has its middle converted, leaving one mangled literal instead of two. Harmless in effect — such an entry is refused either way, and it is refused before anything is written — so it is left alone rather than widened speculatively. One test fixture here is two source lines rather than one because of it, and says so.NNNN#section-N, so citing an RFC section fails the gate even though the role is supported #592 mentions..github/workflows/unit-tests.yml:195adds "The unified diff above shows what moved" unconditionally, including whenrender()raised before any diff was produced. Out of scope for these files and not touched.docs/source/changelog/1.5.0.rststill carries the:rfc:8200section 4.5prose workaround in several bullets that predate this fix. The anchored form works now, but those bullets belong to other branches and rewriting them would only manufacture conflicts.