fix(markdown): escape pipes in code spans inside table cells - #85
fix(markdown): escape pipes in code spans inside table cells#85codeAnqiang-ma wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
1 issue found across 3 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/render/markdown/inline.rs">
<violation number="1" location="src/render/markdown/inline.rs:238">
P1: When code content already has an odd number of backslashes before `|`, this replacement makes the run even and the generated GFM row can still split at that pipe. Add a backslash only when the existing run is even, and cover code such as `a \| b` with a regression test.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
| pub(crate) fn push_code_span(text: &str, ctx: InlineContext, out: &mut String) { | ||
| let text = text.replace('\n', " "); | ||
| // A raw pipe splits a GFM table cell even inside a code span. | ||
| let text = if ctx == InlineContext::TableCell { text.replace('|', "\\|") } else { text }; |
There was a problem hiding this comment.
P1: When code content already has an odd number of backslashes before |, this replacement makes the run even and the generated GFM row can still split at that pipe. Add a backslash only when the existing run is even, and cover code such as a \| b with a regression test.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/render/markdown/inline.rs, line 238:
<comment>When code content already has an odd number of backslashes before `|`, this replacement makes the run even and the generated GFM row can still split at that pipe. Add a backslash only when the existing run is even, and cover code such as `a \| b` with a regression test.</comment>
<file context>
@@ -232,8 +232,10 @@ fn render_text_run(
+pub(crate) fn push_code_span(text: &str, ctx: InlineContext, out: &mut String) {
let text = text.replace('\n', " ");
+ // A raw pipe splits a GFM table cell even inside a code span.
+ let text = if ctx == InlineContext::TableCell { text.replace('|', "\\|") } else { text };
let fence = backtick_fence(&text, 1);
let pad = if text.starts_with('`') || text.ends_with('`') { " " } else { "" };
</file context>
| let text = if ctx == InlineContext::TableCell { text.replace('|', "\\|") } else { text }; | |
| let text = if ctx == InlineContext::TableCell { | |
| let mut escaped = String::with_capacity(text.len()); | |
| let mut backslashes = 0; | |
| for c in text.chars() { | |
| if c == '|' && backslashes % 2 == 0 { | |
| escaped.push('\\'); | |
| } | |
| escaped.push(c); | |
| backslashes = if c == '\\' { backslashes + 1 } else { 0 }; | |
| } | |
| escaped | |
| } else { | |
| text | |
| }; |
There was a problem hiding this comment.
Thanks for the careful look — I checked this against the GFM reference implementation and against GitHub itself, and the row cannot split there: GFM's cell scanning is not parity-based. The cell scanner (ext_scanners.re L32–L35, table_cell = (escaped_char|[^|\r\n])+) matches greedily, so a pipe preceded by any backslash never ends a cell, and unescape_pipes() then strips exactly one backslash before each |. So for code text a \| b, the emitted `a \\| b` renders back as <code>a \| b</code> — an exact round-trip.
Verified via GitHub's own renderer (gh api /markdown):
| A | B |
| --- | --- |
| `one \| two` | X |
| `one \\| two` | Y |Both rows keep two columns, and the cells come back as <code>one | two</code> and <code>one \| two</code>. pulldown-cmark matches GitHub exactly; comrak also keeps every row intact.
The parity variant would instead leave a \| b unescaped, and GFM unescapes \| → | inside code spans too (spec §4.10, example 200), so the backslash would be silently dropped — on every renderer I tested (GitHub, comrak, pulldown-cmark, marked, micromark).
For completeness: marked and micromark do split on `one \\| two` — their cell splitters count backslash parity and deviate from cmark-gfm here. Under those parsers a backslash directly before a pipe inside a code span is unrepresentable either way (escaped, the row splits; unescaped, the backslash is lost), so I kept the encoding that GitHub and the reference implementation render exactly.
Added a regression test covering a \| b in d6fdf04.
Co-authored-by: Cursor <cursoragent@cursor.com>
Fixes #84.
What
|is escaped in table cells for plain text, emphasis, and URLs, but not for code spans, so a code span containing a pipe splits the GFM row and silently drops every column to its right.push_code_span()took noInlineContext, so it could not know it was rendering inside a table cell. This gives it the context and escapes pipes there, matching whatescape_text()already does atescape.rs:82and whatformat_url()does under the comment// Raw pipes split GFM table cells.(escape.rs:151).Per the GFM spec (§4.10 Tables, Example 200), escaping applies inside inline spans:
| b|az |→<td>b <code>|</code> az</td>.Why this way
The escape is applied in
push_code_spanrather than at the call sites so both paths that reach it are covered by one change — inline code runs (inline.rs:208) and cell-levelBlock::CodeBlock(table.rs:169). Behaviour outsideInlineContext::TableCellis untouched, so paragraph-level code spans are unchanged.The escape happens before
backtick_fence()so the fence is computed on the text as emitted, and\|adds no backticks that could affect fence width.Diff
3 files, +34/−3 — 4 production lines and 2 regression tests. No reformatting, no unrelated changes.
Tests
Two regression tests named after the existing
url_pipes_cannot_split_table_cells, placed next to it:code_span_pipes_cannot_split_table_cells(styled run) andcode_block_pipes_cannot_split_table_cells(cell-levelCodeBlock).Both fail before the change and pass after:
No snapshot churn —
git statusis clean apart from the three files in this diff. The fixture corpus has no table cell containing code and a pipe, which is why this was never caught.End-to-end on a minimal
.epubwith<code>a | b</code>and<pre>ls | wc -l</pre>in a two-column table:Rendered through marked 18 with
gfm: true, before → after:Note on clippy
cargo clippy --workspace --all-targets --all-features -- -D warningsfails on my machine with 3collapsible_iferrors insrc/formats/rtf/tables.rs:293,302,422. These are pre-existing and unrelated — I verified they reproduce identically on a clean checkout of4e3089bwith the stash popped, and that file is not in this diff. It looks like a lint that newer clippy (1.95.0) flags but the toolchain CI pinned at release time did not. I left it alone to keep this diff minimal; happy to send a separate PR if you want it cleaned up.Not run locally: the node, wasm, and python binding jobs. This change is confined to the Rust Markdown renderer and does not touch any binding surface.
This change was made with AI assistance; I reproduced the bug, reviewed the patch, and ran every test above locally myself.
Summary by cubic
Escapes pipe characters in code spans inside GFM table cells so rows are not split. Before: a code span with | in a cell emitted a raw pipe and dropped columns to the right; now the pipe is emitted as | and the table structure is preserved. Existing backslashes before | are preserved by emitting \| so round-tripping remains correct.
push_code_spannow takes anInlineContextand escapes|only whenctx == TableCell; other contexts are unchanged.CodeBlockin table cells are handled.Written for commit d6fdf04. Summary will update on new commits.