Repository: raphlinus/pulldown-cmark (rust)

Issue:
Problems with escaping pipes in tables
There doesn't seem to be a way to escape pipes inside a backtick string inside a table.  Example:

```markdown
| 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):

<img width="103" alt="image" src="https://user-images.githubusercontent.com/43198/122294431-e2d15b00-ceac-11eb-8415-c73ef64ba5a5.png">


This is complicated since inside a backtick string [per the commonmark spec](https://spec.commonmark.org/0.29/#example-338), "All backslashes are treated literally".  The GHFM spec has an escape hatch [here](https://github.github.com/gfm/#example-200) 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 `&vert;`.


Pull request description:
Add special handling for backslashed table pipes
Fixes #356

This is an alternative version of https://github.com/raphlinus/pulldown-cmark/pull/691 that follows GitHub's behavior more closely (it gives `\|` higher precedence in the parser than any other backslash escape).

Compare with the rendering in [GitHub].

[GitHub]: https://gist.github.com/notriddle/c027512ee849f12098fec3a3256c89d3

Patch:
diff --git a/Cargo.lock b/Cargo.lock
index 9c158f8..4538294 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -565,7 +565,7 @@ name = "pulldown-cmark"
 version = "0.9.2"
 dependencies = [
  "bincode",
- "bitflags 2.3.3",
+ "bitflags 1.3.2",
  "criterion",
  "getopts",
  "html5ever",
diff --git a/Cargo.toml b/Cargo.toml
index 93fb491..0ec2d5e 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -8,7 +8,6 @@ repository = "https://github.com/raphlinus/pulldown-cmark"
 keywords = ["markdown", "commonmark"]
 categories = ["text-processing"]
 edition = "2021"
-rust-version = "1.64" # Update README.md and azure-pipelines.yml if this changes.
 readme = "README.md"
 exclude = ["/third_party/**/*", "/tools/**/*", "/specs/**/*", "/fuzzer/**/*", "/azure-pipelines.yml"]
 
@@ -56,7 +55,7 @@ name = "broken-link-callbacks"
 required-features = ["html"]
 
 [dependencies]
-bitflags = "2"
+bitflags = "1.3"
 unicase = "2.6"
 memchr = "2.5"
 getopts = { version = "0.2", optional = true }
diff --git a/specs/table.txt b/specs/table.txt
index 1fce4f3..9bdd9c8 100644
--- a/specs/table.txt
+++ b/specs/table.txt
@@ -449,3 +449,100 @@ a\
 <tr><td>d</td><td>e</td></tr>
 </tbody></table>
 ````````````````````````````````
+
+
+As a special case, pipes in inline code in tables are escaped
+with backslashes.
+
+The parsing rule for CommonMark is that block structures are parsed before
+inline structures are. Normally, this means backslashes aren't allowed to have
+any effect on block structures at all. Tables do consider backslashes, but
+not the same way inline syntax does: they have higher precedence than anything
+else, as-if they were parsed in a completely separate pass.
+
+This rule should be identical to GitHub's. See
+<https://gist.github.com/notriddle/c027512ee849f12098fec3a3256c89d3>
+for what they do. This is totally different from Pandoc's `commonmark_x`,
+for example, and from many other Markdown parsers, because of some weird
+corner cases it creates.
+
+```````````````````````````````` example
+| Description | Test case |
+|-------------|-----------|
+| Single      | `\`       |
+| Double      | `\\`      |
+| Basic test  | `\|`      |
+| Basic test 2| `\|\|\`   |
+| Basic test 3| `x\|y\|z\`|
+| Not pipe    | `\.`      |
+| Combo       | `\.\|\`   |
+| Extra       | `\\\.`    |
+| Wait, what? | `\\|`     |
+| Wait, what? | `\\\|`    |
+| Wait, what? | `\\\\|`   |
+| Wait, what? | `\\\\\|`  |
+| Wait, what? |          \|
+| Wait, what? |         \\|
+| Wait, what? |        \\\|
+| Wait, what?x|          \|x
+| Wait, what?x|         \\|x
+| Wait, what?x|        \\\|x
+.
+<table><thead><tr><th>Description</th><th>Test case</th></tr></thead><tbody>
+<tr><td>Single</td><td><code>\</code></td></tr>
+<tr><td>Double</td><td><code>\\</code></td></tr>
+<tr><td>Basic test</td><td><code>|</code></td></tr>
+<tr><td>Basic test 2</td><td><code>||\</code></td></tr>
+<tr><td>Basic test 3</td><td><code>x|y|z\</code></td></tr>
+<tr><td>Not pipe</td><td><code>\.</code></td></tr>
+<tr><td>Combo</td><td><code>\.|\</code></td></tr>
+<tr><td>Extra</td><td><code>\\\.</code></td></tr>
+<tr><td>Wait, what?</td><td><code>\|</code></td></tr>
+<tr><td>Wait, what?</td><td><code>\\|</code></td></tr>
+<tr><td>Wait, what?</td><td><code>\\\|</code></td></tr>
+<tr><td>Wait, what?</td><td><code>\\\\|</code></td></tr>
+<tr><td>Wait, what?</td><td>|</td></tr>
+<tr><td>Wait, what?</td><td>|</td></tr>
+<tr><td>Wait, what?</td><td>\|</td></tr>
+<tr><td>Wait, what?x</td><td>|x</td></tr>
+<tr><td>Wait, what?x</td><td>|x</td></tr>
+<tr><td>Wait, what?x</td><td>\|x</td></tr>
+</tbody></table>
+````````````````````````````````
+
+
+Example with code blocks in the table's header.
+
+```````````````````````````````` example
+| Single | `\|` |
+|--|--|
+| Single | `\|` |
+
+
+| Double | `\\|` |
+|--|--|
+| Double | `\\|` |
+
+
+| Double Twice | `\\|\\|` |
+|--|--|
+| Double Twice | `\\|\\|` |
+
+
+| Triple | `\\\|` |
+|--|--|
+| Triple | `\\\|` |
+.
+<table><thead><tr><th>Single</th><th><code>|</code></th></tr></thead><tbody>
+<tr><td>Single</td><td><code>|</code></td></tr>
+</tbody></table>
+<table><thead><tr><th>Double</th><th><code>\|</code></th></tr></thead><tbody>
+<tr><td>Double</td><td><code>\|</code></td></tr>
+</tbody></table>
+<table><thead><tr><th>Double Twice</th><th><code>\|\|</code></th></tr></thead><tbody>
+<tr><td>Double Twice</td><td><code>\|\|</code></td></tr>
+</tbody></table>
+<table><thead><tr><th>Triple</th><th><code>\\|</code></th></tr></thead><tbody>
+<tr><td>Triple</td><td><code>\\|</code></td></tr>
+</tbody></table>
+````````````````````````````````
diff --git a/src/firstpass.rs b/src/firstpass.rs
index 7249399..faa43a0 100644
--- a/src/firstpass.rs
+++ b/src/firstpass.rs
@@ -606,6 +606,10 @@ impl<'a, 'b> FirstPass<'a, 'b> {
                             });
                             begin_text = ix + 1 + count;
                             LoopInstruction::ContinueAndSkip(count)
+                        } else if ix + 2 < bytes_len && bytes[ix + 2] == b'|' && TableParseMode::Active == mode {
+                            // To parse `\\|`, discard the backslashes and parse the `|` that follows it.
+                            begin_text = ix + 2;
+                            LoopInstruction::ContinueAndSkip(2)
                         } else {
                             begin_text = ix + 1;
                             LoopInstruction::ContinueAndSkip(1)
@@ -705,7 +709,9 @@ impl<'a, 'b> FirstPass<'a, 'b> {
                     _ => LoopInstruction::ContinueAndSkip(0),
                 },
                 b'|' => {
-                    if let TableParseMode::Active = mode {
+                    if ix != 0 && bytes[ix - 1] == b'\\' {
+                        LoopInstruction::ContinueAndSkip(0)
+                    } else if let TableParseMode::Active = mode {
                         LoopInstruction::BreakAtWith(ix, None)
                     } else {
                         last_pipe_ix = ix;
diff --git a/src/lib.rs b/src/lib.rs
index 36e2136..521b85a 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -130,8 +130,6 @@ pub enum MetadataBlockKind {
 }
 
 /// Tags for elements that can contain other elements.
-/// Note that variants are in the same order than in `TagEnd`, so the
-/// matching variant can be compared using `start_tag as u32 == end_tag as u32`.
 #[derive(Clone, Debug, PartialEq)]
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
 pub enum Tag<'a> {
@@ -199,9 +197,6 @@ pub enum Tag<'a> {
     MetadataBlock(MetadataBlockKind),
 }
 
-/// The end of a `Tag`.
-/// Note that variants are in the same order than in `Tag`, so the
-/// matching variant can be compared using `start_tag as u32 == end_tag as u32`.
 #[derive(Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Hash, Debug)]
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
 pub enum TagEnd {
@@ -363,7 +358,6 @@ pub enum Alignment {
 bitflags::bitflags! {
     /// Option struct containing flags for enabling extra features
     /// that are not part of the CommonMark spec.
-    #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)]
     pub struct Options: u32 {
         const ENABLE_TABLES = 1 << 1;
         /// GitHub-compatible footnote syntax.
diff --git a/src/parse.rs b/src/parse.rs
index 53014aa..c760f26 100644
--- a/src/parse.rs
+++ b/src/parse.rs
@@ -812,6 +812,11 @@ impl<'input, 'callback> Parser<'input, 'callback> {
 
             while ix != close {
                 let next_ix = self.tree[ix].next.unwrap();
+                let end = if next_ix == close {
+                    span_end
+                } else {
+                    self.tree[next_ix].item.start
+                };
                 if let ItemBody::HardBreak | ItemBody::SoftBreak = self.tree[ix].item.body {
                     if drop_enclosing_whitespace {
                         // check whether break should be ignored
@@ -838,12 +843,26 @@ impl<'input, 'callback> Parser<'input, 'callback> {
                         new_buf.push(' ');
                         buf = Some(new_buf);
                     }
+                } else if self.tree.is_in_table() && self.text[self.tree[ix].item.start..end].contains("|") {
+                    for (i, c) in bytes[self.tree[ix].item.start..end].into_iter().copied().enumerate() {
+                        if c != b'|' {
+                            continue;
+                        }
+                        let pipe_position = self.tree[ix].item.start + i;
+                        let buf = if let Some(ref mut buf) = buf {
+                            buf.push_str(&self.text[self.tree[ix].item.start..pipe_position]);
+                            buf
+                        } else {
+                            let mut new_buf = String::with_capacity(pipe_position - span_start);
+                            new_buf.push_str(&self.text[span_start..pipe_position]);
+                            buf.insert(new_buf)
+                        };
+                        let ends_in_backslash = buf.as_bytes().last() == Some(&b'\\');
+                        let trim = if ends_in_backslash { 1 } else { 0 };
+                        buf.truncate(buf.len() - trim);
+                        buf.push_str(&self.text[pipe_position..end]);
+                    }
                 } else if let Some(ref mut buf) = buf {
-                    let end = if next_ix == close {
-                        span_end
-                    } else {
-                        self.tree[ix].item.end
-                    };
                     buf.push_str(&self.text[self.tree[ix].item.start..end]);
                 }
                 ix = next_ix;
@@ -953,6 +972,31 @@ impl<'a> Tree<Item> {
             });
         }
     }
+    /// Returns true if the current node is inside a table.
+    ///
+    /// If `cur` is an ItemBody::Table, it would return false,
+    /// but since the `TableRow` and `TableHead` and `TableCell`
+    /// are children of the table, anything doing inline parsing
+    /// doesn't need to care about that.
+    pub(crate) fn is_in_table(&self) -> bool {
+        fn might_be_in_table(item: &Item) -> bool {
+            item.body.is_inline() || matches!(
+                item.body,
+                | ItemBody::TableHead
+                | ItemBody::TableRow
+                | ItemBody::TableCell
+            )
+        }
+        for &ix in self.walk_spine().rev() {
+            if matches!(self[ix].item.body, ItemBody::Table(_)) {
+                return true;
+            }
+            if !might_be_in_table(&self[ix].item) {
+                return false;
+            }
+        }
+        false
+    }
 }
 
 #[derive(Copy, Clone, Debug)]
diff --git a/third_party/GitHub/gfm_table.txt b/third_party/GitHub/gfm_table.txt
index 1c1e592..3e67179 100644
--- a/third_party/GitHub/gfm_table.txt
+++ b/third_party/GitHub/gfm_table.txt
@@ -83,7 +83,7 @@ inline spans:
 </thead>
 <tbody>
 <tr>
-<td>b <code>\|</code> az</td>
+<td>b <code>|</code> az</td>
 </tr>
 <tr>
 <td>b <strong>|</strong> im</td>


Test patch:
diff --git a/tests/suite/gfm_table.rs b/tests/suite/gfm_table.rs
index c45d730..1ab0f19 100644
--- a/tests/suite/gfm_table.rs
+++ b/tests/suite/gfm_table.rs
@@ -68,7 +68,7 @@ fn gfm_table_test_3() {
 </thead>
 <tbody>
 <tr>
-<td>b <code>\|</code> az</td>
+<td>b <code>|</code> az</td>
 </tr>
 <tr>
 <td>b <strong>|</strong> im</td>
diff --git a/tests/suite/table.rs b/tests/suite/table.rs
index c0ab703..6e13dad 100644
--- a/tests/suite/table.rs
+++ b/tests/suite/table.rs
@@ -349,3 +349,89 @@ fn table_test_16() {
 
     test_markdown_html(original, expected, false, false, false);
 }
+
+#[test]
+fn table_test_17() {
+    let original = r##"| Description | Test case |
+|-------------|-----------|
+| Single      | `\`       |
+| Double      | `\\`      |
+| Basic test  | `\|`      |
+| Basic test 2| `\|\|\`   |
+| Basic test 3| `x\|y\|z\`|
+| Not pipe    | `\.`      |
+| Combo       | `\.\|\`   |
+| Extra       | `\\\.`    |
+| Wait, what? | `\\|`     |
+| Wait, what? | `\\\|`    |
+| Wait, what? | `\\\\|`   |
+| Wait, what? | `\\\\\|`  |
+| Wait, what? |          \|
+| Wait, what? |         \\|
+| Wait, what? |        \\\|
+| Wait, what?x|          \|x
+| Wait, what?x|         \\|x
+| Wait, what?x|        \\\|x
+"##;
+    let expected = r##"<table><thead><tr><th>Description</th><th>Test case</th></tr></thead><tbody>
+<tr><td>Single</td><td><code>\</code></td></tr>
+<tr><td>Double</td><td><code>\\</code></td></tr>
+<tr><td>Basic test</td><td><code>|</code></td></tr>
+<tr><td>Basic test 2</td><td><code>||\</code></td></tr>
+<tr><td>Basic test 3</td><td><code>x|y|z\</code></td></tr>
+<tr><td>Not pipe</td><td><code>\.</code></td></tr>
+<tr><td>Combo</td><td><code>\.|\</code></td></tr>
+<tr><td>Extra</td><td><code>\\\.</code></td></tr>
+<tr><td>Wait, what?</td><td><code>\|</code></td></tr>
+<tr><td>Wait, what?</td><td><code>\\|</code></td></tr>
+<tr><td>Wait, what?</td><td><code>\\\|</code></td></tr>
+<tr><td>Wait, what?</td><td><code>\\\\|</code></td></tr>
+<tr><td>Wait, what?</td><td>|</td></tr>
+<tr><td>Wait, what?</td><td>|</td></tr>
+<tr><td>Wait, what?</td><td>\|</td></tr>
+<tr><td>Wait, what?x</td><td>|x</td></tr>
+<tr><td>Wait, what?x</td><td>|x</td></tr>
+<tr><td>Wait, what?x</td><td>\|x</td></tr>
+</tbody></table>
+"##;
+
+    test_markdown_html(original, expected, false, false, false);
+}
+
+#[test]
+fn table_test_18() {
+    let original = r##"| Single | `\|` |
+|--|--|
+| Single | `\|` |
+
+
+| Double | `\\|` |
+|--|--|
+| Double | `\\|` |
+
+
+| Double Twice | `\\|\\|` |
+|--|--|
+| Double Twice | `\\|\\|` |
+
+
+| Triple | `\\\|` |
+|--|--|
+| Triple | `\\\|` |
+"##;
+    let expected = r##"<table><thead><tr><th>Single</th><th><code>|</code></th></tr></thead><tbody>
+<tr><td>Single</td><td><code>|</code></td></tr>
+</tbody></table>
+<table><thead><tr><th>Double</th><th><code>\|</code></th></tr></thead><tbody>
+<tr><td>Double</td><td><code>\|</code></td></tr>
+</tbody></table>
+<table><thead><tr><th>Double Twice</th><th><code>\|\|</code></th></tr></thead><tbody>
+<tr><td>Double Twice</td><td><code>\|\|</code></td></tr>
+</tbody></table>
+<table><thead><tr><th>Triple</th><th><code>\\|</code></th></tr></thead><tbody>
+<tr><td>Triple</td><td><code>\\|</code></td></tr>
+</tbody></table>
+"##;
+
+    test_markdown_html(original, expected, false, false, false);
+}