Skip to content

Fix line attribution inside included markdown being off by one - #1184

Open
dchaudhari7177 wants to merge 1 commit into
executablebooks:masterfrom
dchaudhari7177:fix/include-line-attribution
Open

Fix line attribution inside included markdown being off by one#1184
dchaudhari7177 wants to merge 1 commit into
executablebooks:masterfrom
dchaudhari7177:fix/include-line-attribution

Conversation

@dchaudhari7177

Copy link
Copy Markdown

Fixes #1165.

Cause

MockIncludeDirective.run calls:

self.renderer.nested_render_text(file_content, startline + 1, ...)

nested_render_text adds that lineno to the 0-based token maps, and _render_tokens then applies its own 0-based → 1-based + 1. startline is already the 0-based offset of the first included line, so adding one here reports every warning inside an included file one line too low. Exactly as diagnosed in the issue — a one-token change.

The existing fixture encoded the bug

tests/test_renderers/fixtures/mock_include_errors.md expected:

tmpdir/bad.md:2: (WARNING/2) Unknown interpreted text role "a".

while bad.md is written as "{a}b" — a single line. That expectation is now :1.

Regression cases

The issue asked for these before touching the line, so the fixture gains five more, each with the offending role in a paragraph of its own (MyST attributes an inline warning to its enclosing block's start line, which is worth being deliberate about here):

case file role on expected
plain include bad_line3.md 3 :3
include after content in the parent bad_line3.md 3 :3
:start-line: 2 bad_skipped.md 4 :4
front matter stripped bad_frontmatter.md 5 :5
nested include bad_inner.md via bad_outer.md 3 :3

All six cases fail on master and pass with the change — I checked by reverting only mocking.py and leaving the tests in place.

The nested case is worth calling out: before this change it appeared correct for a role sharing a paragraph with the preceding line, because the two errors cancelled. Putting the role in its own paragraph shows it was off by one like the rest.

:heading-offset: was also probed and is unaffected (heading offsetting shifts levels, not maps), as is front-matter stripping — the front_matter token is popped before the maps are adjusted, so it shifts nothing.

Test run

1240 passed, 18 skipped. Three failures are pre-existing on an unmodified checkout in this environment (test_cmdline[40-linkify], test_extended_syntaxes, test_extended_syntaxes_text) — I diffed the FAILED sets before and after, and those three are identical on both sides.

🤖 Generated with Claude Code

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 executablebooks#1165
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

1 participant