Merge pull request #244 from bplump/fix-table-heading-with-pipes-and-…

…trailing-spaces Fix tables with leading/trailing header pipes and trailing spaces
commonmark · Sep 6, 2021 · e3a50fc · e3a50fc
2 parents 726d369 + cb692b9
commit e3a50fc
Show file tree

Hide file tree

Showing 2 changed files with 54 additions and 3 deletions.
diff --git a/...ext-gfm-tables/src/main/java/org/commonmark/ext/gfm/tables/internal/TableBlockParser.java b/...ext-gfm-tables/src/main/java/org/commonmark/ext/gfm/tables/internal/TableBlockParser.java
@@ -122,14 +122,22 @@ private TableCell parseCell(SourceLine cell, int column, InlineParser inlinePars
     private static List<SourceLine> split(SourceLine line) {
         CharSequence row = line.getContent();
         int nonSpace = Parsing.skipSpaceTab(row, 0, row.length());
-        int cellStart = row.charAt(nonSpace) == '|' ? nonSpace + 1 : nonSpace;
+        int cellStart = nonSpace;
+        int cellEnd = row.length();
+        if (row.charAt(nonSpace) == '|') {
+            // This row has leading/trailing pipes - skip the leading pipe
+            cellStart = nonSpace + 1;
+            // Strip whitespace from the end but not the pipe or we could miss an empty ("||") cell
+            int nonSpaceEnd = Parsing.skipSpaceTabBackwards(row, row.length() - 1, cellStart + 1);
+            cellEnd = nonSpaceEnd + 1;
+        }
         List<SourceLine> cells = new ArrayList<>();
         StringBuilder sb = new StringBuilder();
-        for (int i = cellStart; i < row.length(); i++) {
+        for (int i = cellStart; i < cellEnd; i++) {
             char c = row.charAt(i);
             switch (c) {
                 case '\\':
-                    if (i + 1 < row.length() && row.charAt(i + 1) == '|') {
+                    if (i + 1 < cellEnd && row.charAt(i + 1) == '|') {
                         // Pipe is special for table parsing. An escaped pipe doesn't result in a new cell, but is
                         // passed down to inline parsing as an unescaped pipe. Note that that applies even for the `\|`
                         // in an input like `\\|` - in other words, table parsing doesn't support escaping backslashes.

diff --git a/commonmark-ext-gfm-tables/src/test/java/org/commonmark/ext/gfm/tables/TablesTest.java b/commonmark-ext-gfm-tables/src/test/java/org/commonmark/ext/gfm/tables/TablesTest.java
@@ -235,6 +235,49 @@ public void pipesOnOutside() {
                 "</table>\n");
     }
 
+    @Test
+    public void pipesOnOutsideWhitespaceAfterHeader() {
+        assertRendering("|Abc|Def| \n|---|---|\n|1|2|", "<table>\n" +
+                "<thead>\n" +
+                "<tr>\n" +
+                "<th>Abc</th>\n" +
+                "<th>Def</th>\n" +
+                "</tr>\n" +
+                "</thead>\n" +
+                "<tbody>\n" +
+                "<tr>\n" +
+                "<td>1</td>\n" +
+                "<td>2</td>\n" +
+                "</tr>\n" +
+                "</tbody>\n" +
+                "</table>\n");
+    }
+
+    @Test
+    public void pipesOnOutsideZeroLengthHeaders() {
+        // This is literally what someone has done IRL - it helped to expose
+        // an issue with parsing the last header cell correctly
+        assertRendering("||center header||\n" +
+                        "-|-------------|-\n" +
+                        "1|      2      |3",
+                "<table>\n" +
+                "<thead>\n" +
+                "<tr>\n" +
+                "<th></th>\n" +
+                "<th>center header</th>\n" +
+                "<th></th>\n" +
+                "</tr>\n" +
+                "</thead>\n" +
+                "<tbody>\n" +
+                "<tr>\n" +
+                "<td>1</td>\n" +
+                "<td>2</td>\n" +
+                "<td>3</td>\n" +
+                "</tr>\n" +
+                "</tbody>\n" +
+                "</table>\n");
+    }
+
     @Test
     public void inlineElements() {
         assertRendering("*Abc*|Def\n---|---\n1|2", "<table>\n" +