fix: report inline columns as written inside an escaped table cell - #405
Merged
Conversation
comrak unescapes a table cell before it parses the cell's inlines, so every `\|` written earlier in the cell drops a byte from the string the inline parser measures against. Every inline after the escape is recorded one column to the left of where it was written, and one more for each further escape, and the rules forwarded that to the report: the caret landed on the wrong character. `Document` now records, for each line where the two columns part company, which column comrak's column was written at, and `written_position` maps one to the other. The table is built from the cells' own `sourcepos`, which comrak takes from the raw line and so does not shift, and only for a document that contains an escape at all. MD033, MD034, MD037, MD038 and MD039 report through it. MD034 and MD037 add their own offsets into the unescaped text on top of a start that has already been shifted; mapping the position they arrive at covers both. MD038 keeps measuring its span against the columns comrak reports, since those are the ones that describe `literal`, and maps only the report. `\\|` is an escaped backslash rather than an escaped pipe, and comrak drops nothing for it, so the escapes are found by walking the backslash run rather than by matching the pair. Closes #403 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011xqGos7Q8MiBm6G59sh4FV
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #405 +/- ##
========================================
Coverage 99.79% 99.80%
========================================
Files 72 72
Lines 7715 8082 +367
========================================
+ Hits 7699 8066 +367
Misses 16 16 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
The line lookup and the cell slice were written as guarded lookups for positions a table cell cannot hold — a line outside the document, a column off a character boundary — so nothing could reach them and the two `continue`s went uncovered. The cell is taken as one expression now, and a slice that is somehow not there carries no escape either, which is the same answer the guards gave. Sizing the table to the cells it was built from rather than to the line means a second escaped cell has to extend it, so a row with one in each cell is what covers that, and it also pins the shift restarting at every cell rather than accumulating across the row. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011xqGos7Q8MiBm6G59sh4FV
Four findings from review of the previous commits. comrak unescapes the paragraph it splits off a table's header row as well, so an inline written above a table carried the same shift and had nothing to correct it. That paragraph is the one that ends on the line directly above a table it is a sibling of — a table can only begin by converting an open paragraph, so without a blank line between them the two came from one block, and with one there is a line between them. Each of its lines is measured from its own offset, so each is walked whole and shifted by the escapes written on it alone. The mapping stopped at the last column comrak reports, which is short of the region's end by the number of bytes dropped from it. A rule that counts an exclusive end off a span running to the end of a cell names one of those, and got it back unshifted; MD034 on a bare URL at the end of a cell is the case. The columns left over now carry the region's whole shift, which is what applies past its content. MD033 looked the position up for every node in the document rather than for the two arms that report one, and the table is an `FxHashMap` now, as everywhere else in the crate. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011xqGos7Q8MiBm6G59sh4FV
Three findings from review. A column is put back on the line by the character comrak reports at it, so a column that names the byte *after* a span answers for whatever is there. MD034's end is one such: with a `\|` written directly after a bare URL, the end followed the escape past the URL it was meant to bound. It now maps the URL's last byte and steps one past that, which is the same column it always was outside a table. That also removes what the leftover-column fill was standing in for. The fill only reached an exclusive end that ran to a region's end, and an end landing anywhere else went through the ordinary map and picked up the escape — so it was covering one case of a problem that belonged to the caller. The table was a `Vec` of every column from the start of the line to the end of the last region on it, so it was sized by how long the lines were rather than by how many escapes they held. A line is now the regions comrak unescaped, each one a start column and the columns dropped from it, and a column is answered for by the last region beginning at or before it. A region reaches to where the next begins rather than to where its content stops, so the columns at a region's end still carry its shift without being written down. Recording a row's cells together is what lets a cell after an escaped one say that the shift stops there. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011xqGos7Q8MiBm6G59sh4FV
Two documentation findings from review. `written_position` claimed a position outside the region it names comes back unchanged, which is the opposite of what the lookup does: a region reaches to where the next begins, so the columns at its end carry its shift. Said the other way round now, along with why — a column there belongs next to the one before it rather than back where it started. The pipe is the only escape this reaches, and the doc did not say so. Any other backslash escape earlier in the same text also moves the columns MD034 and MD037 count off it, because both index a literal `CommonMark` had already resolved the escape in, and comrak never shifted the column they add to — so nothing here can find it. That is a defect of its own rather than a limit of this correction, and is #406; MD034, MD037 and the changelog entry now say where the guarantee stops. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011xqGos7Q8MiBm6G59sh4FV
The doc had the pipe as the one escape comrak resolves before it measures, which holds only where it is unescaped ahead of the inline parser — inside a table cell, or in the paragraph split off a header row. Written anywhere else it is resolved by that parser like any other escape, so `see x\|y http://www.example.com/` reports column 9 for a URL at column 10, and nothing here can find that either. #406 says so too now, so the two do not disagree about whose the pipe is. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011xqGos7Q8MiBm6G59sh4FV
akiomik
added a commit
that referenced
this pull request
Sep 6, 2026
) `CommonMark` resolves a backslash escape before a text node's `literal` is built, so `\.` is two bytes on the line and one there. MD034 and MD037 counted their offsets off that literal and added them to the node's start column, which comrak records against the line: every offset past an escape named a column one to the left of the character it was written at, and one more for each further escape. MD037 fared worse than shifted — an escaped `*` or `_` reaches the literal as a bare one, so `x \* y ** b ** z` was reported at the escaped marker, which is neither of the two markers the line has. The two rules want different things, because their defects are different. MD034 goes on scanning the literal: what a bare URL is is a question about the text a reader is given, and only its columns were wrong. `Document::written_column_of` walks an offset of the literal along the line — the two run together a character at a time and part company only at an escape, whose backslash is a byte of the line the literal does not have — and answers with the column it lands on. Both ends go through it, the end asked for as the byte after the URL rather than as its last byte stepped past: a step of one is the width of that byte, which is one only where it is a single byte written as itself. MD037 searches the line, because no arithmetic on a column can undo an escape the literal has already resolved. It searches the line with each escape masked out a byte for a byte, so an escaped marker is invisible to the search and every column it reports is still the column the byte is at. Searching the line alone would not do it: only the opening marker of `\s\*\s.+\*` and the three like it is anchored to whitespace, which a backslash cannot be, and the backslash left on the line is a byte for `.+` besides. Masking is also what gets `\\*` right, that being an escaped backslash and then a marker. #405's correction runs first for both, a position from inside a table cell being measured against the unescaped cell rather than against the line. What it corrects and what this does are the two halves of one report: the position of the node, and the offsets counted off inside it. comrak measures a node from the byte its literal begins with, and a byte written escaped is a column further along than the escape that wrote it, so the slice is taken from the escape rather than from the column comrak gives. Without it the slice is the literal's twin rather than its source and every escape after it is read a byte early. It is also what puts an escaped marker at the start of a node where the mask can see it, which is where `` `c`\* a * b `` was reported at a marker its author escaped. The line is used only where it reads back as the source the literal was built from, which is checked rather than assumed. comrak measures the inlines after one that spans two lines from a line behind, and the slice then holds text from elsewhere in the document — the gitlab corpus has one, and against the line it read as a bare URL the document does not have there. A character reference is resolved into the literal the way an escape is, and naming the character `&` stands for takes the whole HTML5 table. Both fall back to what the rules did before this. MD037 also trims the whitespace anchoring a match off whichever end carries it, and off by that character's own width: `\s` is a tab as much as a space, and two bytes for a no-break space. A tab put the report on the tab rather than on the marker after it, and a wider space put a column inside the character. `http\://example.com` reads as a bare URL still, as it does on `main`, and GFM does not autolink it. Which of the two strings a rule scans is the wrong lever for that — scanning the line hands back the `http://ex` of `http://ex\_ample.com/` as a URL of its own — and #408 has it. 378 tests pass. The acceptance corpus, the gitlab corpus and this repository's own Markdown report identically before and after, so nothing outside an escape moved, and `src/document.rs`, MD034 and MD037 keep 100% line coverage. Closes #406.
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 #403.
The defect
comrak unescapes a table cell before it parses the cell's inlines, so every
\|written earlier in the cell drops a byte from the string the inline parser
measures
sourceposagainst. Every inline after the escape is recorded onecolumn to the left of where it was written, and one more for each further
escape. The rules forwarded that unchanged, so the caret landed on the wrong
character.
The inline starts at column 8 on every row. Before, mado reported column 7 for
MD038, MD037, MD033 and MD034, and column 8 for MD039 — which #403 read as MD039
being unaffected. It is not: MD039 reports the link's text, which is at column
9, so it was shifted by one like the rest and only looked right against the
issue's yardstick. All five now report the column the character was written at.
\|inside the inline shifts its end one further than its start, and both areput back independently.
The fix
Documentrecords, for each line where the two columns part company, whichcolumn comrak's column was written at, and
written_positionmaps one to theother. The table is built from the cells' own
sourcepos, which comrak takesfrom the raw line and so does not shift, and a document with no
\|in itbuilds nothing.
comrak unescapes one other region the same way — the paragraph it splits off a
table's header row (
table.rs:299) — so text written directly above a tablecarried the shift too. That paragraph is the one ending on the line directly
above a table it is a sibling of: a table can only begin by converting an open
paragraph, so without a blank line between them the two came from one block, and
with one there is a line between them. It is corrected the same way, per line.
A column is put back on the line by the character comrak reports at it, so a
column naming the byte after a span answers for whatever is there. MD034's end
is one of those: with a
\|directly after a bare URL, the end followed theescape past the URL it was meant to bound. MD034 maps the URL's last byte and
steps one past that instead, which is the same column it always was outside a
table.
A line is stored as the regions comrak unescaped — each a start column and the
columns dropped from it — so the memory is the escapes rather than the length of
the line. A column is answered for by the last region beginning at or before it,
and a region reaches to where the next begins rather than to where its content
stops, so a column at a region's end still carries its shift. A row's cells are
recorded together, which is how a cell after an escaped one says the shift stops
there.
MD033, MD034, MD037, MD038 and MD039 report through it:
start that has already been shifted. Mapping the position they arrive at
covers both, because that position is in the same coordinates comrak reported.
the ones that describe
literal— and maps only the report. Its existing testasserted the shifted column and named this issue; it now asserts the written
one.
\\|is an escaped backslash rather than an escaped pipe, and comrak dropsnothing for it, so the escapes are found by walking the backslash run rather
than by matching the pair. Matching pairs shifted a column that was never
shifted, which is what the
written_position_with_escaped_backslashtest pins.Verification
cargo test --all-features --workspace: 370 tests pass, including newtable-cell tests in each of the five rules and eight for
written_position.cargo clippy --all-targets --all-features --workspace -- -D warningsandcargo fmt --all --checkare clean.scripts/acceptance/data/markdownlint/test/rule_testsand over this repository's own Markdown, before and after: byte-identical
output, so nothing outside an escaped table cell moved.
an escape written after the inline, and multibyte cell content all report the
column as written.
Where the correction stops
The pipe is the only escape this reaches, and only where it is resolved ahead of
the inline parser rather than by it — inside a table cell, or in the paragraph
split off a header row. Written anywhere else,
\|is resolved like any otherescape, and so is every
\<punctuation>everywhere: the columns MD034 and MD037count off a text node come from a literal
CommonMarkhad already resolved theescape in, and comrak never shifted the column they add to, so
written_positionhas nothing to find.That is a separate defect, present on
mainand unchanged here; it is filed as#406, and
written_position, MD034, MD037 and the changelog entry say where theguarantee stops.
Left open
#403 also asks whether the fix belongs upstream in comrak. This one is in mado,
where the reported position is assembled; it does not change what comrak
records, so an upstream fix later would make
written_positiona no-op ratherthan conflict with it.
🤖 Generated with Claude Code
https://claude.ai/code/session_011xqGos7Q8MiBm6G59sh4FV