Repository navigation
🐛 FIX: exact source lines in {include}d files - #1169
Merged
Merged
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
MockIncludeDirective.run passed `startline + 1` to nested_render_text, but that lineno is added to the 0-based token maps *before* _render_tokens applies its own 0-based to 1-based conversion. The offset of the first included line is `startline`, already 0-based, so every warning raised inside an included file was reported one line too low. The existing "Error in include file" fixture encoded the off-by-one: bad.md holds the role on line 1 and the expectation read `bad.md:2`. Fixes #1165 (cherry picked from commit 8cee240)
Fixture cases for each line-attribution branch in `MockIncludeDirective`: a multi-line `start-after` marker (YAML option form), negative `start-line` (`-2`, and `-100` beyond the file's length), a CRLF file with `start-after`, `start-after` with `end-before`, a marker that ends the file (no content, no warning), and a nested include using `start-line` followed by a warning in the outer file. Each fix hunk is load-bearing: without the negative `start-line` resolution 2 cases fail, without the newline count for `start-after` 5 fail, and with `startline + 1` restored 14 fail.
`test_include_source_line_attribution` split the warning on `":"`, which breaks on Windows paths (`C:\...`), and its first failing assert hid the rest. Every case it checked (plain, `start-line`, `start-after`, nested) is now a data-driven case in `mock_include_errors.md`.
chrisjsewell
changed the title
{include}d files
chrisjsewell
marked this pull request as ready for review
November 10, 2026 10:50
This was referenced Nov 10, 2026
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.
Closes #1165, closes #1216.
Warnings raised inside an
{include}d Markdown file now cite the file's true lines::4):startline + 1was passed tonested_render_text, which already gets the0-based to 1-based conversion in
_render_tokens(Line attribution inside{include}d markdown is off by one #1165);:start-after:advanced the line offset by the marker's character index instead of the lines it skips ({include} with :start-after: adds a character index to the line number #1216);:start-line:was not resolved to the line it counts from the end.The
:literal:/:code:branches are unchanged (they keep docutils' line numbering).The topmatter half of the original draft (#1167) was dropped: #1167 lands via #1197.
Tests are data-driven cases in
tests/test_renderers/fixtures/mock_include_errors.md:#1184's cases (cherry-picked, authorship kept), #1217's two
:start-after:cases with the corrected expectation(line 4, not 5), and edge cases (multi-line marker, negative
start-line, CRLF,start-afterwithend-before,a marker ending the file, nested includes with
start-line). 14 of its 17 cases fail on master.The earlier Python test, which split warnings on
":"and so broke on Windows drive letters, is removed.Thanks to @Sanjays2402 (#1166), @dchaudhari7177 (#1184) and @vishalkaay (#1217), whose fixes and cases are folded in.
Not in scope: under Sphinx, the warning still names the parent page with the included file's line (#1175).