Skip to content

🐛 FIX: Correct included source line attribution - #1166

Closed
Sanjays2402 wants to merge 1 commit into
executablebooks:masterfrom
Sanjays2402:fix/include-warning-lines
Closed

Sanjays2402 wants to merge 1 commit into
executablebooks:masterfrom
Sanjays2402:fix/include-warning-lines

Conversation

@Sanjays2402

Copy link
Copy Markdown
Contributor

Fixes #1165.

Included Markdown passed startline + 1 to a renderer that already converts zero-based token maps to one-based source lines. Passing startline keeps diagnostics on the actual line; regression cases cover plain, :start-line:, and nested includes.

Tested with python -m pytest -q (1160 passed, 11 skipped).

Included Markdown applied a one-based offset before token maps were converted
to one-based source lines, shifting every diagnostic down by one line.

Pass the zero-based start line to the nested renderer and cover plain,
start-line, and nested includes.

Tests: python -m pytest -q
Fixes executablebooks#1165
@chrisjsewell

Copy link
Copy Markdown
Member

Thank you @Sanjays2402, and apologies for the long wait. Yours was the first PR for #1165, with exactly the right one-token fix. We've landed it as part of #1169 (1915654), which also had to fix :start-after: counting characters instead of lines (#1216) and negative :start-line:, in the same few lines of MockIncludeDirective.run. That made the separate PRs conflict, so we folded the cluster into one change. You're credited as a co-author on the merge commit. Closing as superseded. Thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Line attribution inside {include}d markdown is off by one

2 participants