Merge pull request #381 from commonmark/tables-without-blank-line-before

Parse tables without blank line before

Author: robinst

CHANGELOG.md

@@ -10,6 +10,8 @@ with the exception that 0.x versions can break between minor versions.
 ### Added
 - More documentation with examples for `Node` classes
 ### Changed
+- GitHub tables: Tables are now parsed even if there's no blank line before the
+  table heading, matching GitHub's behavior.
 ### Fixed
 - `MarkdownRenderer`: Fix precedence for `nodeRendererFactory`: Factories passed
   to the builder can now override rendering for core node types.

commonmark-ext-gfm-tables/src/main/java/org/commonmark/ext/gfm/tables/internal/TableBlockParser.java

@@ -274,17 +274,17 @@ public static class Factory extends AbstractBlockParserFactory {
         @Override
         public BlockStart tryStart(ParserState state, MatchedBlockParser matchedBlockParser) {
             List<SourceLine> paragraphLines = matchedBlockParser.getParagraphLines().getLines();
-            if (paragraphLines.size() == 1 && Characters.find('|', paragraphLines.get(0).getContent(), 0) != -1) {
+            if (paragraphLines.size() >= 1 && Characters.find('|', paragraphLines.get(paragraphLines.size() - 1).getContent(), 0) != -1) {
                 SourceLine line = state.getLine();
                 SourceLine separatorLine = line.substring(state.getIndex(), line.getContent().length());
                 List<TableCellInfo> columns = parseSeparator(separatorLine.getContent());
                 if (columns != null && !columns.isEmpty()) {
-                    SourceLine paragraph = paragraphLines.get(0);
+                    SourceLine paragraph = paragraphLines.get(paragraphLines.size() - 1);
                     List<SourceLine> headerCells = split(paragraph);
                     if (columns.size() >= headerCells.size()) {
                         return BlockStart.of(new TableBlockParser(columns, paragraph))
                                 .atIndex(state.getIndex())
-                                .replaceActiveBlockParser();
+                                .replaceParagraphLines(1);
                     }
                 }
             }

commonmark-ext-gfm-tables/src/test/java/org/commonmark/ext/gfm/tables/TablesTest.java

@@ -1,8 +1,7 @@
 package org.commonmark.ext.gfm.tables;
 
 import org.commonmark.Extension;
-import org.commonmark.node.Node;
-import org.commonmark.node.SourceSpan;
+import org.commonmark.node.*;
 import org.commonmark.parser.IncludeSourceSpans;
 import org.commonmark.parser.Parser;
 import org.commonmark.renderer.html.AttributeProvider;
@@ -79,11 +78,6 @@ public void separatorNeedsPipes() {
         assertRendering("Abc|Def\n|--- ---", "<p>Abc|Def\n|--- ---</p>\n");
     }
 
-    @Test
-    public void headerMustBeOneLine() {
-        assertRendering("No\nAbc|Def\n---|---", "<p>No\nAbc|Def\n---|---</p>\n");
-    }
-
     @Test
     public void oneHeadNoBody() {
         assertRendering("Abc|Def\n---|---", "<table>\n" +
@@ -702,6 +696,26 @@ public void danglingPipe() {
                 "<p>|</p>\n");
     }
 
+    @Test
+    public void interruptsParagraph() {
+        assertRendering("text\n" +
+                "|a  |\n" +
+                "|---|\n" +
+                "|b  |", "<p>text</p>\n" +
+                "<table>\n" +
+                "<thead>\n" +
+                "<tr>\n" +
+                "<th>a</th>\n" +
+                "</tr>\n" +
+                "</thead>\n" +
+                "<tbody>\n" +
+                "<tr>\n" +
+                "<td>b</td>\n" +
+                "</tr>\n" +
+                "</tbody>\n" +
+                "</table>\n");
+    }
+
     @Test
     public void attributeProviderIsApplied() {
         AttributeProviderFactory factory = new AttributeProviderFactory() {
@@ -835,6 +849,36 @@ public void sourceSpans() {
         assertThat(bodyRow3Cell2.getSourceSpans()).isEqualTo(List.of());
     }
 
+    @Test
+    public void sourceSpansWhenInterrupting() {
+        var parser = Parser.builder()
+                .extensions(EXTENSIONS)
+                .includeSourceSpans(IncludeSourceSpans.BLOCKS_AND_INLINES)
+                .build();
+        var document = parser.parse("a\n" +
+                "bc\n" +
+                "|de|\n" +
+                "|---|\n" +
+                "|fg|");
+
+        var paragraph = (Paragraph) document.getFirstChild();
+        var text = (Text) paragraph.getFirstChild();
+        assertThat(text.getLiteral()).isEqualTo("a");
+        assertThat(text.getNext()).isInstanceOf(SoftLineBreak.class);
+        var text2 = (Text) text.getNext().getNext();
+        assertThat(text2.getLiteral()).isEqualTo("bc");
+
+        assertThat(paragraph.getSourceSpans()).isEqualTo(List.of(
+                SourceSpan.of(0, 0, 0, 1),
+                SourceSpan.of(1, 0, 2, 2)));
+
+        var table = (TableBlock) document.getLastChild();
+        assertThat(table.getSourceSpans()).isEqualTo(List.of(
+                SourceSpan.of(2, 0, 5, 4),
+                SourceSpan.of(3, 0, 10, 5),
+                SourceSpan.of(4, 0, 16, 4)));
+    }
+
     @Override
     protected String render(String source) {
         return RENDERER.render(PARSER.parse(source));

commonmark/src/main/java/org/commonmark/internal/BlockStartImpl.java

@@ -9,6 +9,7 @@ public class BlockStartImpl extends BlockStart {
     private int newIndex = -1;
     private int newColumn = -1;
     private boolean replaceActiveBlockParser = false;
+    private int replaceParagraphLines = 0;
 
     public BlockStartImpl(BlockParser... blockParsers) {
         this.blockParsers = blockParsers;
@@ -30,6 +31,10 @@ public boolean isReplaceActiveBlockParser() {
         return replaceActiveBlockParser;
     }
 
+    int getReplaceParagraphLines() {
+        return replaceParagraphLines;
+    }
+
     @Override
     public BlockStart atIndex(int newIndex) {
         this.newIndex = newIndex;
@@ -48,4 +53,12 @@ public BlockStart replaceActiveBlockParser() {
         return this;
     }
 
+    @Override
+    public BlockStart replaceParagraphLines(int lines) {
+        if (!(lines >= 1)) {
+            throw new IllegalArgumentException("Lines must be >= 1");
+        }
+        this.replaceParagraphLines = lines;
+        return this;
+    }
 }

commonmark/src/main/java/org/commonmark/internal/DocumentParser.java

@@ -270,9 +270,15 @@ private void parseLine(String ln, int inputIndex) {
             }
 
             List<SourceSpan> replacedSourceSpans = null;
-            if (blockStart.isReplaceActiveBlockParser()) {
-                Block replacedBlock = prepareActiveBlockParserForReplacement();
-                replacedSourceSpans = replacedBlock.getSourceSpans();
+            if (blockStart.getReplaceParagraphLines() >= 1 || blockStart.isReplaceActiveBlockParser()) {
+                var activeBlockParser = getActiveBlockParser();
+                if (activeBlockParser instanceof ParagraphParser) {
+                    var paragraphParser = (ParagraphParser) activeBlockParser;
+                    var lines = blockStart.isReplaceActiveBlockParser() ? Integer.MAX_VALUE : blockStart.getReplaceParagraphLines();
+                    replacedSourceSpans = replaceParagraphLines(lines, paragraphParser);
+                } else if (blockStart.isReplaceActiveBlockParser()) {
+                    replacedSourceSpans = prepareActiveBlockParserForReplacement(activeBlockParser);
+                }
             }
 
             for (BlockParser newBlockParser : blockStart.getBlockParsers()) {
@@ -498,24 +504,23 @@ private OpenBlockParser deactivateBlockParser() {
         return openBlockParsers.remove(openBlockParsers.size() - 1);
     }
 
-    private Block prepareActiveBlockParserForReplacement() {
-        // Note that we don't want to parse inlines, as it's getting replaced.
-        BlockParser old = deactivateBlockParser().blockParser;
+    private List<SourceSpan> replaceParagraphLines(int lines, ParagraphParser paragraphParser) {
+        // Remove lines from paragraph as the new block is using them.
+        // If all lines are used, this also unlinks the Paragraph block.
+        var sourceSpans = paragraphParser.removeLines(lines);
+        // Close the paragraph block parser, which will finalize it.
+        closeBlockParsers(1);
+        return sourceSpans;
+    }
 
-        if (old instanceof ParagraphParser) {
-            ParagraphParser paragraphParser = (ParagraphParser) old;
-            // Collect any link reference definitions. Note that replacing the active block parser is done after a
-            // block parser got the current paragraph content using MatchedBlockParser#getContentString. In case the
-            // paragraph started with link reference definitions, we parse and strip them before the block parser gets
-            // the content. We want to keep them.
-            // If no replacement happens, we collect the definitions as part of finalizing blocks.
-            addDefinitionsFrom(paragraphParser);
-        }
+    private List<SourceSpan> prepareActiveBlockParserForReplacement(BlockParser blockParser) {
+        // Note that we don't want to parse inlines here, as it's getting replaced.
+        deactivateBlockParser();
 
         // Do this so that source positions are calculated, which we will carry over to the replacing block.
-        old.closeBlock();
-        old.getBlock().unlink();
-        return old.getBlock();
+        blockParser.closeBlock();
+        blockParser.getBlock().unlink();
+        return blockParser.getBlock().getSourceSpans();
     }
 
     private Document finalizeAndProcess() {

commonmark/src/main/java/org/commonmark/internal/HeadingParser.java

@@ -60,7 +60,7 @@ public BlockStart tryStart(ParserState state, MatchedBlockParser matchedBlockPar
                 if (!paragraph.isEmpty()) {
                     return BlockStart.of(new HeadingParser(setextHeadingLevel, paragraph))
                             .atIndex(line.getContent().length())
-                            .replaceActiveBlockParser();
+                            .replaceParagraphLines(paragraph.getLines().size());
                 }
             }
 

commonmark/src/main/java/org/commonmark/internal/LinkReferenceDefinitionParser.java

@@ -10,6 +10,7 @@
 import org.commonmark.parser.beta.Scanner;
 
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
 
 /**
@@ -102,6 +103,14 @@ State getState() {
         return state;
     }
 
+    List<SourceSpan> removeLines(int lines) {
+        var removedSpans = Collections.unmodifiableList(new ArrayList<>(
+                sourceSpans.subList(Math.max(sourceSpans.size() - lines, 0), sourceSpans.size())));
+        removeLast(lines, paragraphLines);
+        removeLast(lines, sourceSpans);
+        return removedSpans;
+    }
+
     private boolean startDefinition(Scanner scanner) {
         // Finish any outstanding references now. We don't do this earlier because we need addSourceSpan to have been
         // called before we do it.
@@ -269,6 +278,16 @@ private void finishReference() {
         title = null;
     }
 
+    private static <T> void removeLast(int n, List<T> list) {
+        if (n >= list.size()) {
+            list.clear();
+        } else {
+            for (int i = 0; i < n; i++) {
+                list.remove(list.size() - 1);
+            }
+        }
+    }
+
     enum State {
         // Looking for the start of a definition, i.e. `[`
         START_DEFINITION,

commonmark/src/main/java/org/commonmark/internal/ParagraphParser.java

@@ -79,4 +79,8 @@ public void parseInlines(InlineParser inlineParser) {
     public SourceLines getParagraphLines() {
         return linkReferenceDefinitionParser.getParagraphLines();
     }
+
+    public List<SourceSpan> removeLines(int lines) {
+        return linkReferenceDefinitionParser.removeLines(lines);
+    }
 }

commonmark/src/main/java/org/commonmark/parser/block/BlockStart.java

@@ -10,18 +10,59 @@ public abstract class BlockStart {
     protected BlockStart() {
     }
 
+    /**
+     * Result for when there is no block start.
+     */
     public static BlockStart none() {
         return null;
     }
 
+    /**
+     * Start block(s) with the specified parser(s).
+     */
     public static BlockStart of(BlockParser... blockParsers) {
         return new BlockStartImpl(blockParsers);
     }
 
+    /**
+     * Continue parsing at the specified index.
+     *
+     * @param newIndex the new index, see {@link ParserState#getIndex()}
+     */
     public abstract BlockStart atIndex(int newIndex);
 
+    /**
+     * Continue parsing at the specified column (for tab handling).
+     *
+     * @param newColumn the new column, see {@link ParserState#getColumn()}
+     */
     public abstract BlockStart atColumn(int newColumn);
 
+    /**
+     * @deprecated use {@link #replaceParagraphLines(int)} instead; please raise an issue if that doesn't work for you
+     * for some reason.
+     */
+    @Deprecated
     public abstract BlockStart replaceActiveBlockParser();
 
+    /**
+     * Replace a number of lines from the current paragraph (as returned by
+     * {@link MatchedBlockParser#getParagraphLines()}) with the new block.
+     * <p>
+     * This is useful for parsing blocks that start with normal paragraphs and only have special marker syntax in later
+     * lines, e.g. in this:
+     * <pre>
+     * Foo
+     * ===
+     * </pre>
+     * The <code>Foo</code> line is initially parsed as a normal paragraph, then <code>===</code> is parsed as a heading
+     * marker, replacing the 1 paragraph line before. The end result is a single Heading block.
+     * <p>
+     * Note that source spans from the replaced lines are automatically added to the new block.
+     *
+     * @param lines the number of lines to replace (at least 1); use {@link Integer#MAX_VALUE} to replace the whole
+     *              paragraph
+     */
+    public abstract BlockStart replaceParagraphLines(int lines);
+
 }

commonmark/src/main/java/org/commonmark/parser/block/MatchedBlockParser.java

@@ -12,7 +12,8 @@ public interface MatchedBlockParser {
     BlockParser getMatchedBlockParser();
 
     /**
-     * Returns the current paragraph lines if the matched block is a paragraph.
+     * Returns the current paragraph lines if the matched block is a paragraph. If you want to use some or all of the
+     * lines for starting a new block instead, use {@link BlockStart#replaceParagraphLines(int)}.
      *
      * @return paragraph content or an empty list
      */