🐛 FIX: topmatter config merging, and source lines in included files#1169
Draft
chrisjsewell wants to merge 2 commits into
Draft
🐛 FIX: topmatter config merging, and source lines in included files#1169chrisjsewell wants to merge 2 commits into
chrisjsewell wants to merge 2 commits into
Conversation
Two fixes:
- `merge_file_level` re-assigned the raw topmatter value after
validation, clobbering values set by converting validators - most
visibly, a `heading_slug_func` preset name crashed at every heading
with "'str' object is not callable", and `enable_extensions` survived
as a raw list instead of a set. Validated values are now kept, and
fields marked global-only are rejected in topmatter with a
`myst.topmatter` warning instead of being silently (mis)applied.
- every warning inside an `{include}`d markdown file was attributed one
line too low: the include directive passed `startline + 1` to the
nested renderer, double-counting its 0-based to 1-based conversion.
Additionally `start-after` advanced the line offset by *characters*
rather than lines, and a negative `start-line` was passed through
unresolved; both now map exactly.
Closes #1165, closes #1167
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two independent fixes. Closes #1165, closes #1167.
Enforcing
global_onlyin topmatter (below) removes two undocumented but previously functioning behaviours, found by the review's old-vs-new probes:inventoriesin topmatter worked per-document in docutils-standalone (inv:links resolved against it). Now: amyst.topmatterwarning and the field is ignored (the documented routes —docutils.confmyst-inventories, CLI flag — are unaffected). Sphinx was never affected (intersphinx).update_mathjax: falsein topmatter half-worked under sphinx (suppressed thetex2jax_ignoreclasses on that page, though never the MathJax config override). Now warned-and-ignored.Both fields are already marked global-only in the generated configuration docs, the break is loud rather than silent, and blessing them as per-document would create sphinx/docutils divergence — so this PR enforces. If you'd rather deprecate first (apply + warn "will be ignored in future"), that's a two-line change; say the word. Note also that any document carrying a global-only key in topmatter now emits a warning where it was previously silent, so strict (
-W) builds may surface them (suppressible viamyst.topmatter).Topmatter config merging (#1167)
merge_file_levelre-assigned the raw topmatter value after validation, clobbering values that converting validators had set:heading_slug_func: githubin topmatter crashed at every heading ('str' object is not callable) — now it warns once as global-only.enable_extensionssurvived as a raw list instead of the validated set;url_schemesin list form crashed the publish outright, and its dict form skipped URL templating (rawrefuri="wiki:Hobbit"). All now keep their validated/converted values.Fields with
global_onlymetadata are rejected in topmatter with a warning naming the field; everything else keeps the validator's converted value via a before/after identity check (with a documented contract note for future validator authors).Source lines in included files (#1165)
Every warning inside an
{include}d markdown file was attributed one line too low (startline + 1double-counted the renderer's 0-based→1-based conversion). Also fixed while there:start-afteradvanced the offset by characters rather than lines, and negativestart-linepassed through unresolved. All shapes now map exactly — plain,start-line(including negative),start-after(including a multi-line marker and marker-at-EOF), combined withend-before, and nested includes — withmock_include_errors.md's regenerated expectation hand-verified against the included file. The:literal:/:code:branches are untouched (docutils line parity from #1164 preserved).Review
Adversarial + behavior-regression pass with old-vs-new probes against master: a 7-page sphinx include-project renders byte-identical HTML with only line-number corrections in warnings/gettext locations (each hand-verified against the physical file); all five global-only fields probed individually in both docutils and sphinx paths (results above); adversarial include options (negative/beyond-EOF start-line, CRLF, multi-line markers) crash-free with exact lines. Two pre-existing items noted, not touched: sphinx console warnings show the parent page's path with the included file's line (
location=(env.docname, line)— warning-taxonomy territory), and a few othernested_render_textcallers share the same off-by-one family (substitutions already carry a TODO).Full suite: 1247 passed.
Generated by Claude Code