Problems with escaping pipes in tables

[ehuss]

There doesn't seem to be a way to escape pipes inside a backtick string inside a table. Example:

| My Table |
|----------|
| `Foo \| Bar` |

Returns a code event with the literal backslash still in it: Code(Borrowed("Foo \\| Bar")).

However, if you don't include the backslash, it breaks the table (and indeed, that is what GitHub renders here):

This is complicated since inside a backtick string per the commonmark spec, "All backslashes are treated literally". The GHFM spec has an escape hatch here where they specify "Include a pipe in a cell’s content by escaping it, including inside other inline spans". So there is a little conflict there, but I think if pulldown-cmark wants to follow GHFM tables, it should probably handle escapes the same way.

A workaround for now is to use HTML, such as <code>Foo \| Bar </code> or <code>Foo &vert; Bar </code>

EDIT: Original issue below, the rest of the issues below have been fixed.

---

I'm running into some issues with escaping pipes which makes it difficult to use them in tables.

Escaping only works at the beginning inside backticks

Given the text `\|` I get the expected event Code(Borrowed("|")).

Given the text `foo \|` I get Code(Borrowed("foo \\|")), notice the unexpected backslash in the output. Escaping a pipe only works if it is the very first thing inside backticks. I would expect it to work anywhere inside backticks.

Escaping inside code block does not work

Given the text <code>\|</code> results in exactly the same output (that is, with the backslash). I would expect the backslash to be removed (and is how GitHub interprets it). This is an issue because if you try to use <code>|</code> inside a table, the pipe is interpreted as a column separator. Adding the backslash prevents that, but results in an unwanted backslash in the output.

For now I think I'll be able to work around this using |.

[oberien]

Citing from the Commonmark Spec:

Backslash escapes do not work in code blocks, code spans, autolinks, or raw HTML

As such I think your first example (`\|`) is a bug, because it should result in <code>\|</code>, which is also what the dingus does.

[raphlinus]

This is a bug, yes. I took a little look at the code and got confused; I see a bunch of special case handling of backslash/code fence interactions, but am not convinced it's the right thing. The reason `foo \|` is being rendered correctly is somewhat tricky - the tree is missing the backslash, but make_code_span makes a Code item based on a contiguous span from the start of the first item in the line to the last (when a backslash is the first character, the first item is a text beginning after the backslash).

The sense I get is that the logic should look like this:

In the first pass, \ followed by ASCII punctuation produces a Backslash ItemBody, and the character after it is always text, even if it is `. So the sequence \`` should produce Backslash, text `, and MaybeCode(1). I think this subsumes the existing preceded_by_backslash logic.

The make_code_span function rewrites Backslash to output a backslash character. But in item_to_event, Backslash produces nothing (this could be an empty text, or there could be additional logic to suppress even that).

I'm not sure how using | is a workaround; it should produce the text "|", which will get rendered with the ampersand unescaped. It's certainly possible there are bugs here as well.

[marcusklaas]

It seems the sequence \`` shouldn't produce a MaybeCode(1) since it can be used to close a 2-backtick delimited codespan. For example:

``this is code\``

should produce <code>this is code\</code>. I believe this is not directly related to the escaping of other punctuation though.

It may be necessary to reintroduce ItemBody::Backslash, but this has the downside of adding nodes to the tree that have to be filtered out during event iteration. If this problem only pops up at the start of inline code spans, altering that routine could be more feasible.

[ehuss]

Hm, perhaps I was a bit confused too. I guess the question is if it should follow some of the rules of GitHub flavor tables. Specifically https://github.github.com/gfm/#example-200 says "Include a pipe in a cell’s content by escaping it, including inside other inline spans:", which gives an example of `\|` being converted to <code>|</code>. This is distinctly different from the commonmark spec.

The workaround I meant is <code>&vert;</code> which AFAIK commonmark and github treat as I would expect (no escaping), and seems to work fine in pulldown-cmark.

[marcusklaas]

That's a great insight @ehuss. I guess we should parse differently inside tables..

edit: Reintroducing ItemBody::Backslash increasingly seems like the right solution to me as well. I guess that's what its purpose was after all.

[mity]

You might be interested in this discussion too: https://talk.commonmark.org/t/parsing-strategy-for-tables/2027

Also note the table parsing in GFM has some history. If I remember it correctly, originally GFM (before migrating to cmark-gfm) used only (unescaped) pipes outside any code span to delimit table cells. I.e. code spans were parsed with higher precedence then the table cells.

MD4C was implemented that way too and it still sticks to that behavior as I see that better for the purposes noted in this thread.

The downside of that approach is that (arguably) it is against the parse-blocks-before-spans rule of the CommonMark specs, but my POV is that given that table cell cannot span over multiple lines, we can see table cell as a special span (from Markdown syntax POV).

[raphlinus]

I take back what I said about Backslash above, the example provided by Marcus above is compelling. Aside from tables, I think we can fix this by looking backwards from the first text for a backslash.

Tables look like they'll require more consideration, and also thanks to the original poster for bringing this to our attention.

[marcusklaas]

Additional troublesome case:

> `
> `

which incorrectly produces

<blockquote>
<p><code>&gt;</code></p>
</blockquote>

[marcusklaas]

@mity: as always, those are excellent considerations. I've played around with a bit, and I don't think a similar strategy will work for pulldown-cmark as it has split parsing blocks (including table cells) and inline elements like code spans to different phases.

The alternative, which is GFM's approach, has the very surprising behavior that `\|` is rendered differently inside tables (and the code block is not "verbatim", which it is everywhere else).

Unfortunately, when adhering to both blocks-before-spans and code-always-verbatim, it seems there is no way to produce <td><code>|</code></td>..

[ehuss]

I'm finding a few more problems with backslash escapes.

Markdown	pulldown-cmark	Expected
`\`	Code(Borrowed(""))	Code(Borrowed("\\"))
`\\`	Code(Borrowed("\\"))	Code(Borrowed("\\\\"))

I suspect it is related to the same issue here.

This idiom is used several times in Rust documentation (like https://doc.rust-lang.org/nightly/std/path/index.html, the reference, and rust-by-example).

[raphlinus]

I'd like to get a fix for this in soon, but maybe we can split the table stuff out into a separate bug, as I'm not sure there's a consensus on how to handle it.

[marcusklaas]

I have a fix for the non table stuff!On Jul 11, 2019 08:51, Raph Levien <notifications@github.com> wrote:I'd like to get a fix for this in soon, but maybe we can split the table stuff out into a separate bug, as I'm not sure there's a consensus on how to handle it. —You are receiving this because you commented.Reply to this email directly, view it on GitHub, or mute the thread.