Skip to content

🐛 FIX: count lines, not characters, for {include} :start-after: - #1217

Closed
vishalkaay wants to merge 1 commit into
executablebooks:masterfrom
vishalkaay:fix-include-start-after-line
Closed

vishalkaay wants to merge 1 commit into
executablebooks:masterfrom
vishalkaay:fix-include-start-after-line

Conversation

@vishalkaay

Copy link
Copy Markdown
Contributor

Fixes #1216

:start-after: added the marker's character index to startline, so lines in the included content (and their warnings) were shifted by the number of characters before the marker. It now adds the number of lines consumed before the slice.

Two cases in mock_include_errors.md cover :start-after: alone and together with :start-line:; without the fix they report lines 31 and 23 for a role on line 5.

The `:start-after:` option added the character index of the end of the
marker to `startline`, which counts lines. Every line of the included
content, and every warning raised in it, was shifted by the number of
characters before the marker.

Add the number of newlines consumed before the slice instead, and cover
`:start-after:` alone and combined with `:start-line:` in the include
error fixtures.

Fixes executablebooks#1216
chrisjsewell added a commit that referenced this pull request Nov 9, 2026
The two cases from #1217 (`:start-after:` alone, and with `:start-line:`),
with the expectation corrected from `bad_after.md:5` to `:4`: the `:5`
encoded the separate #1165 off-by-one, and the paragraph holding the role
starts on line 4 of the file.

Co-authored-by: vishalkaay <vishalr23@iitk.ac.in>
chrisjsewell added a commit that referenced this pull request Nov 10, 2026
Warnings raised inside an `{include}`d Markdown file now cite the file's
true lines. `startline + 1` was passed to `nested_render_text`, which
already applies the 0-based to 1-based conversion, so every line was
reported one line late (#1165); `:start-after:` advanced the line offset
by the marker's character index instead of the lines it skips (#1216);
and a negative `:start-line:` is now resolved to the line it counts from.

The tests are data-driven cases in `mock_include_errors.md`, including the
cases from #1184 and #1217 and edge cases for multi-line markers, CRLF
files, `end-before`, a marker ending the file and nested includes.

Fixes #1165, fixes #1216

Co-authored-by: dchaudhari7177 <dipakchaudhari171@gmail.com>
Co-authored-by: vishalkaay <vishalr23@iitk.ac.in>
Co-authored-by: Sanjay Santhanam <51058514+Sanjays2402@users.noreply.github.com>
@chrisjsewell

Copy link
Copy Markdown
Member

Thank you @vishalkaay, the line-counting fix for :start-after: is exactly right. One catch: the new fixture expected bad_after.md:5, which bakes in the separate off-by-one from #1165 (every included line was reported one line late). With that fixed, the role's paragraph is correctly reported at line 4, so the two PRs couldn't both land as written. We've landed your fix via #1169 (1915654) together with the #1165 fix, and adopted your two cases with the corrected expectation. You're credited as a co-author on the merge commit. Closing as superseded. Thanks!

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.

{include} with :start-after: adds a character index to the line number

2 participants