Merge pull request #19 from mattermost/MM-55972-59809_tables

MM-55972/MM-59809 Simplify parsing of table delimiters and improve how whitespace in tables is handled
mattermost · Aug 26, 2024 · 238f58c · 238f58c
2 parents 9ebe192 + f842daf
commit 238f58c
Show file tree

Hide file tree

Showing 4 changed files with 231 additions and 59 deletions.
diff --git a/dist/commonmark.js b/dist/commonmark.js
@@ -14134,12 +14134,8 @@
 
     var reLineEnding = /\r\n|\n|\r/;
 
-    var reTableDelimiter = /^[ \t]{0,3}((?:\|[ \t]*)?:?-+:?[ \t]*(?:\|(?:[ \t]*:?-+:?[ \t]*)?)*\|?)$/;
-
     var reTableRow = /^(\|?)(?:(?:\\\||[^|])*\|?)+$/;
 
-    var reTablePipeSpaceEnding = /\|\s+$/;
-
     var MAX_AUTOCOMPLETED_CELLS = 1000;
 
     // Returns true if string contains only space characters.
@@ -14159,10 +14155,6 @@
         }
     };
 
-    var trimSpacesAfterPipe = function(ln) {
-        return ln.replace(reTablePipeSpaceEnding,"|");
-    };
-
     // DOC PARSER
 
     // These are methods of a Parser object, defined below.
@@ -14854,9 +14846,12 @@
         // table
         function(parser, container) {
             if (container.type !== "document") {
+                // @hmhealey We should be able to have a table inside of a list item or block quote to match GitHub
+                // but I'm not sure if the mobile app can render that, so let's not bother with it for now
                 return 0;
             }
 
+
             if (parser.indented) {
                 return 0;
             }
@@ -14866,20 +14861,32 @@
                 return 0;
             }
 
+            const nextColumn = measureNonspaceColumn(parser.nextLine);
+            if (nextColumn - parser.column >= CODE_INDENT || parser.column - nextColumn >= CODE_INDENT) {
+                // the delimiter row must be on the same indentation level as the header row
+                return 0;
+            }
+
+            parser.advanceNextNonspace();
+
             // check for a delimiter first since it's stricter than the header row
-            const nextLine = trimSpacesAfterPipe(parser.nextLine);
-            var delimiterMatch = reTableDelimiter.exec(nextLine);
-            if (!delimiterMatch || delimiterMatch[0].indexOf("|") === -1) {
+            const nextLine = parser.nextLine.trim();
+            const delimiterMatch = reTableRow.exec(nextLine);
+            if (!delimiterMatch) {
                 return 0;
             }
 
-            const currentLine = trimSpacesAfterPipe(parser.currentLine);
-            var headerMatch = reTableRow.exec(currentLine.slice(parser.nextNonspace));
+            const delimiterCells = parseDelimiterRow(delimiterMatch[0]);
+            if (!delimiterCells) {
+                return 0;
+            }
+
+            const currentLine = parser.currentLine.slice(parser.nextNonspace).trim();
+            var headerMatch = reTableRow.exec(currentLine);
             if (!headerMatch) {
                 return 0;
             }
 
-            var delimiterCells = parseTableCells(delimiterMatch[1]);
             var headerCells = parseTableCells(headerMatch[0]);
 
             if (delimiterCells.length !== headerCells.length) {
@@ -14939,6 +14946,23 @@
         }
     ];
 
+    const reValidTableDelimiter = /^:?-+:?$/;
+    const parseDelimiterRow = function(row) {
+        if (row.indexOf("|") === -1) {
+            return null;
+        }
+
+        const cells = parseTableCells(row).map((cell) => cell.trim());
+
+        for (const cell of cells) {
+            if (!reValidTableDelimiter.test(cell)) {
+                return null;
+            }
+        }
+
+        return cells;
+    };
+
     var parseTableCells = function(row) {
         // remove starting pipe to make life easier
         row = row.replace(/^\|/, "");
@@ -15028,6 +15052,27 @@
         this.indented = this.indent >= CODE_INDENT;
     };
 
+    function measureNonspaceColumn(line) {
+        // This code is copied from findNextNonspace above
+        var i = 0;
+        var cols = 0;
+        var c;
+
+        while ((c = line.charAt(i)) !== "") {
+            if (c === " ") {
+                i++;
+                cols++;
+            } else if (c === "\t") {
+                i++;
+                cols += 4 - (cols % 4);
+            } else {
+                break;
+            }
+        }
+
+        return cols;
+    }
+
     // Analyze a line of text and update the document appropriately.
     // We parse markdown text by calling this on each line of input,
     // then finalizing the document.

diff --git a/dist/commonmark.min.js b/dist/commonmark.min.js
diff --git a/lib/blocks.js b/lib/blocks.js
@@ -55,11 +55,9 @@ var reSetextHeadingLine = /^(?:=+|-+)[ \t]*$/;
 
 var reLineEnding = /\r\n|\n|\r/;
 
-var reTableDelimiter = /^[ \t]{0,3}((?:\|[ \t]*)?:?-+:?[ \t]*(?:\|(?:[ \t]*:?-+:?[ \t]*)?)*\|?)$/;
-
 var reTableRow = /^(\|?)(?:(?:\\\||[^|])*\|?)+$/;
 
-var reTablePipeSpaceEnding = /\|\s+$/;
+var reValidTableDelimiter = /^:?-+:?$/;
 
 var MAX_AUTOCOMPLETED_CELLS = 1000;
 
@@ -80,10 +78,6 @@ var peek = function(ln, pos) {
     }
 };
 
-var trimSpacesAfterPipe = function(ln) {
-    return ln.replace(reTablePipeSpaceEnding,"|");
-}
-
 // DOC PARSER
 
 // These are methods of a Parser object, defined below.
@@ -775,9 +769,12 @@ var blockStarts = [
     // table
     function(parser, container) {
         if (container.type !== "document") {
+            // @hmhealey We should be able to have a table inside of a list item or block quote to match GitHub
+            // but I'm not sure if the mobile app can render that, so let's not bother with it for now
             return 0;
         }
 
+
         if (parser.indented) {
             return 0;
         }
@@ -787,20 +784,32 @@ var blockStarts = [
             return 0;
         }
 
+        const nextColumn = measureNonspaceColumn(parser.nextLine);
+        if (Math.abs(nextColumn - parser.column) >= CODE_INDENT) {
+            // the delimiter row must be on the same indentation level as the header row
+            return 0;
+        }
+
+        parser.advanceNextNonspace();
+
         // check for a delimiter first since it's stricter than the header row
-        const nextLine = trimSpacesAfterPipe(parser.nextLine);
-        var delimiterMatch = reTableDelimiter.exec(nextLine);
-        if (!delimiterMatch || delimiterMatch[0].indexOf("|") === -1) {
+        const nextLine = parser.nextLine.trim();
+        const delimiterMatch = reTableRow.exec(nextLine);
+        if (!delimiterMatch) {
             return 0;
         }
 
-        const currentLine = trimSpacesAfterPipe(parser.currentLine);
-        var headerMatch = reTableRow.exec(currentLine.slice(parser.nextNonspace));
+        const delimiterCells = parseDelimiterRow(delimiterMatch[0]);
+        if (!delimiterCells) {
+            return 0;
+        }
+
+        const currentLine = parser.currentLine.slice(parser.nextNonspace).trim();
+        var headerMatch = reTableRow.exec(currentLine);
         if (!headerMatch) {
             return 0;
         }
 
-        var delimiterCells = parseTableCells(delimiterMatch[1]);
         var headerCells = parseTableCells(headerMatch[0]);
 
         if (delimiterCells.length !== headerCells.length) {
@@ -860,6 +869,22 @@ var blockStarts = [
     }
 ];
 
+const parseDelimiterRow = function(row) {
+    if (row.indexOf("|") === -1) {
+        return null;
+    }
+
+    const cells = parseTableCells(row).map((cell) => cell.trim());
+
+    for (const cell of cells) {
+        if (!reValidTableDelimiter.test(cell)) {
+            return null;
+        }
+    }
+
+    return cells;
+}
+
 var parseTableCells = function(row) {
     // remove starting pipe to make life easier
     row = row.replace(/^\|/, "");
@@ -949,6 +974,27 @@ var findNextNonspace = function() {
     this.indented = this.indent >= CODE_INDENT;
 };
 
+function measureNonspaceColumn(line) {
+    // This code is copied from findNextNonspace above
+    var i = 0;
+    var cols = 0;
+    var c;
+
+    while ((c = line.charAt(i)) !== "") {
+        if (c === " ") {
+            i++;
+            cols++;
+        } else if (c === "\t") {
+            i++;
+            cols += 4 - (cols % 4);
+        } else {
+            break;
+        }
+    }
+
+    return cols;
+}
+
 // Analyze a line of text and update the document appropriately.
 // We parse markdown text by calling this on each line of input,
 // then finalizing the document.