Fix tables rendering

commonmark · Sep 24, 2024 · 6e529d4 · 6e529d4
1 parent 1906ee9
commit 6e529d4
Show file tree

Hide file tree

Showing 3 changed files with 88 additions and 79 deletions.
diff --git a/...es/src/main/java/org/commonmark/ext/gfm/tables/internal/TableTextContentNodeRenderer.java b/...es/src/main/java/org/commonmark/ext/gfm/tables/internal/TableTextContentNodeRenderer.java
@@ -23,10 +23,11 @@ public TableTextContentNodeRenderer(TextContentNodeRendererContext context) {
     }
 
     protected void renderBlock(TableBlock tableBlock) {
+        // Render rows tight
+        textContentWriter.pushTight(true);
         renderChildren(tableBlock);
-        if (tableBlock.getNext() != null) {
-            textContentWriter.write("\n");
-        }
+        textContentWriter.popTight();
+        textContentWriter.block();
     }
 
     protected void renderHead(TableHead tableHead) {
@@ -38,33 +39,24 @@ protected void renderBody(TableBody tableBody) {
     }
 
     protected void renderRow(TableRow tableRow) {
-        textContentWriter.line();
         renderChildren(tableRow);
-        textContentWriter.line();
+        textContentWriter.block();
     }
 
     protected void renderCell(TableCell tableCell) {
         renderChildren(tableCell);
-        textContentWriter.write('|');
-        textContentWriter.whitespace();
-    }
-
-    private void renderLastCell(TableCell tableCell) {
-        renderChildren(tableCell);
+        // For the last cell in row, don't render the delimiter
+        if (tableCell.getNext() != null) {
+            textContentWriter.write('|');
+            textContentWriter.whitespace();
+        }
     }
 
     private void renderChildren(Node parent) {
         Node node = parent.getFirstChild();
         while (node != null) {
             Node next = node.getNext();
-
-            // For last cell in row, we dont render the delimiter.
-            if (node instanceof TableCell && next == null) {
-                renderLastCell((TableCell) node);
-            } else {
-                context.render(node);
-            }
-
+            context.render(node);
             node = next;
         }
     }

diff --git a/...ark-ext-gfm-tables/src/test/java/org/commonmark/ext/gfm/tables/TablesTextContentTest.java b/...ark-ext-gfm-tables/src/test/java/org/commonmark/ext/gfm/tables/TablesTextContentTest.java
@@ -2,137 +2,165 @@
 
 import org.commonmark.Extension;
 import org.commonmark.parser.Parser;
+import org.commonmark.renderer.text.LineBreakRendering;
 import org.commonmark.renderer.text.TextContentRenderer;
-import org.commonmark.testutil.RenderingTestCase;
+import org.commonmark.testutil.Asserts;
 import org.junit.Test;
 
 import java.util.Set;
 
-public class TablesTextContentTest extends RenderingTestCase {
+public class TablesTextContentTest {
 
     private static final Set<Extension> EXTENSIONS = Set.of(TablesExtension.create());
     private static final Parser PARSER = Parser.builder().extensions(EXTENSIONS).build();
     private static final TextContentRenderer RENDERER = TextContentRenderer.builder().extensions(EXTENSIONS).build();
 
+    private static final TextContentRenderer COMPACT_RENDERER = TextContentRenderer.builder().extensions(EXTENSIONS).build();
+    private static final TextContentRenderer SEPARATE_RENDERER = TextContentRenderer.builder().extensions(EXTENSIONS)
+            .lineBreakRendering(LineBreakRendering.SEPARATE_BLOCKS).build();
+    private static final TextContentRenderer STRIPPED_RENDERER = TextContentRenderer.builder().extensions(EXTENSIONS)
+            .lineBreakRendering(LineBreakRendering.STRIP).build();
+
     @Test
     public void oneHeadNoBody() {
-        assertRendering("Abc|Def\n---|---", "Abc| Def\n");
+        assertCompact("Abc|Def\n---|---", "Abc| Def");
     }
 
     @Test
     public void oneColumnOneHeadNoBody() {
-        String expected = "Abc\n";
-        assertRendering("|Abc\n|---\n", expected);
-        assertRendering("|Abc|\n|---|\n", expected);
-        assertRendering("Abc|\n---|\n", expected);
+        String expected = "Abc";
+        assertCompact("|Abc\n|---\n", expected);
+        assertCompact("|Abc|\n|---|\n", expected);
+        assertCompact("Abc|\n---|\n", expected);
 
         // Pipe required on separator
-        assertRendering("|Abc\n---\n", "|Abc");
+        assertCompact("|Abc\n---\n", "|Abc");
         // Pipe required on head
-        assertRendering("Abc\n|---\n", "Abc\n|---");
+        assertCompact("Abc\n|---\n", "Abc\n|---");
     }
 
     @Test
     public void oneColumnOneHeadOneBody() {
-        String expected = "Abc\n1\n";
-        assertRendering("|Abc\n|---\n|1", expected);
-        assertRendering("|Abc|\n|---|\n|1|", expected);
-        assertRendering("Abc|\n---|\n1|", expected);
+        String expected = "Abc\n1";
+        assertCompact("|Abc\n|---\n|1", expected);
+        assertCompact("|Abc|\n|---|\n|1|", expected);
+        assertCompact("Abc|\n---|\n1|", expected);
 
         // Pipe required on separator
-        assertRendering("|Abc\n---\n|1", "|Abc\n|1");
+        assertCompact("|Abc\n---\n|1", "|Abc\n|1");
     }
 
     @Test
     public void oneHeadOneBody() {
-        assertRendering("Abc|Def\n---|---\n1|2", "Abc| Def\n1| 2\n");
+        assertCompact("Abc|Def\n---|---\n1|2", "Abc| Def\n1| 2");
     }
 
     @Test
     public void separatorMustNotHaveLessPartsThanHead() {
-        assertRendering("Abc|Def|Ghi\n---|---\n1|2|3", "Abc|Def|Ghi\n---|---\n1|2|3");
+        assertCompact("Abc|Def|Ghi\n---|---\n1|2|3", "Abc|Def|Ghi\n---|---\n1|2|3");
     }
 
     @Test
     public void padding() {
-        assertRendering(" Abc  | Def \n --- | --- \n 1 | 2 ", "Abc| Def\n1| 2\n");
+        assertCompact(" Abc  | Def \n --- | --- \n 1 | 2 ", "Abc| Def\n1| 2");
     }
 
     @Test
     public void paddingWithCodeBlockIndentation() {
-        assertRendering("Abc|Def\n---|---\n    1|2", "Abc| Def\n1| 2\n");
+        assertCompact("Abc|Def\n---|---\n    1|2", "Abc| Def\n1| 2");
     }
 
     @Test
     public void pipesOnOutside() {
-        assertRendering("|Abc|Def|\n|---|---|\n|1|2|", "Abc| Def\n1| 2\n");
+        assertCompact("|Abc|Def|\n|---|---|\n|1|2|", "Abc| Def\n1| 2");
     }
 
     @Test
     public void inlineElements() {
-        assertRendering("*Abc*|Def\n---|---\n1|2", "Abc| Def\n1| 2\n");
+        assertCompact("*Abc*|Def\n---|---\n1|2", "Abc| Def\n1| 2");
     }
 
     @Test
     public void escapedPipe() {
-        assertRendering("Abc|Def\n---|---\n1\\|2|20", "Abc| Def\n1|2| 20\n");
+        assertCompact("Abc|Def\n---|---\n1\\|2|20", "Abc| Def\n1|2| 20");
     }
 
     @Test
     public void alignLeft() {
-        assertRendering("Abc|Def\n:---|---\n1|2", "Abc| Def\n1| 2\n");
+        assertCompact("Abc|Def\n:---|---\n1|2", "Abc| Def\n1| 2");
     }
 
     @Test
     public void alignRight() {
-        assertRendering("Abc|Def\n---:|---\n1|2", "Abc| Def\n1| 2\n");
+        assertCompact("Abc|Def\n---:|---\n1|2", "Abc| Def\n1| 2");
     }
 
     @Test
     public void alignCenter() {
-        assertRendering("Abc|Def\n:---:|---\n1|2", "Abc| Def\n1| 2\n");
+        assertCompact("Abc|Def\n:---:|---\n1|2", "Abc| Def\n1| 2");
     }
 
     @Test
     public void alignCenterSecond() {
-        assertRendering("Abc|Def\n---|:---:\n1|2", "Abc| Def\n1| 2\n");
+        assertCompact("Abc|Def\n---|:---:\n1|2", "Abc| Def\n1| 2");
     }
 
     @Test
     public void alignLeftWithSpaces() {
-        assertRendering("Abc|Def\n :--- |---\n1|2", "Abc| Def\n1| 2\n");
+        assertCompact("Abc|Def\n :--- |---\n1|2", "Abc| Def\n1| 2");
     }
 
     @Test
     public void alignmentMarkerMustBeNextToDashes() {
-        assertRendering("Abc|Def\n: ---|---", "Abc|Def\n: ---|---");
-        assertRendering("Abc|Def\n--- :|---", "Abc|Def\n--- :|---");
-        assertRendering("Abc|Def\n---|: ---", "Abc|Def\n---|: ---");
-        assertRendering("Abc|Def\n---|--- :", "Abc|Def\n---|--- :");
+        assertCompact("Abc|Def\n: ---|---", "Abc|Def\n: ---|---");
+        assertCompact("Abc|Def\n--- :|---", "Abc|Def\n--- :|---");
+        assertCompact("Abc|Def\n---|: ---", "Abc|Def\n---|: ---");
+        assertCompact("Abc|Def\n---|--- :", "Abc|Def\n---|--- :");
     }
 
     @Test
     public void bodyCanNotHaveMoreColumnsThanHead() {
-        assertRendering("Abc|Def\n---|---\n1|2|3", "Abc| Def\n1| 2\n");
+        assertCompact("Abc|Def\n---|---\n1|2|3", "Abc| Def\n1| 2");
     }
 
     @Test
     public void bodyWithFewerColumnsThanHeadResultsInEmptyCells() {
-        assertRendering("Abc|Def|Ghi\n---|---|---\n1|2", "Abc| Def| Ghi\n1| 2| \n");
+        assertCompact("Abc|Def|Ghi\n---|---|---\n1|2", "Abc| Def| Ghi\n1| 2| ");
     }
 
     @Test
     public void insideBlockQuote() {
-        assertRendering("> Abc|Def\n> ---|---\n> 1|2", "«\nAbc| Def\n1| 2\n»");
+        assertCompact("> Abc|Def\n> ---|---\n> 1|2", "«Abc| Def\n1| 2»");
     }
 
     @Test
     public void tableWithLazyContinuationLine() {
-        assertRendering("Abc|Def\n---|---\n1|2\nlazy", "Abc| Def\n1| 2\nlazy| \n");
+        assertCompact("Abc|Def\n---|---\n1|2\nlazy", "Abc| Def\n1| 2\nlazy| ");
+    }
+
+    @Test
+    public void tableBetweenOtherBlocks() {
+        var s = "Foo\n\nAbc|Def\n---|---\n1|2\n\nBar";
+        assertCompact(s, "Foo\nAbc| Def\n1| 2\nBar");
+        assertSeparate(s, "Foo\n\nAbc| Def\n1| 2\n\nBar");
+        assertStripped(s, "Foo Abc| Def 1| 2 Bar");
+    }
+
+    private void assertCompact(String source, String expected) {
+        var doc = PARSER.parse(source);
+        var actualRendering = COMPACT_RENDERER.render(doc);
+        Asserts.assertRendering(source, expected, actualRendering);
+    }
+
+    private void assertSeparate(String source, String expected) {
+        var doc = PARSER.parse(source);
+        var actualRendering = SEPARATE_RENDERER.render(doc);
+        Asserts.assertRendering(source, expected, actualRendering);
     }
 
-    @Override
-    protected String render(String source) {
-        return RENDERER.render(PARSER.parse(source));
+    private void assertStripped(String source, String expected) {
+        var doc = PARSER.parse(source);
+        var actualRendering = STRIPPED_RENDERER.render(doc);
+        Asserts.assertRendering(source, expected, actualRendering);
     }
 }
diff --git a/commonmark/src/test/java/org/commonmark/test/TextContentRendererTest.java b/commonmark/src/test/java/org/commonmark/test/TextContentRendererTest.java
@@ -2,7 +2,6 @@
 
 import org.commonmark.renderer.text.LineBreakRendering;
 import org.commonmark.renderer.text.TextContentRenderer;
-import org.commonmark.node.Node;
 import org.commonmark.parser.Parser;
 import org.commonmark.testutil.Asserts;
 import org.junit.Test;
@@ -11,6 +10,12 @@
 
 public class TextContentRendererTest {
 
+    private static final Parser PARSER = Parser.builder().build();
+    private static final TextContentRenderer COMPACT_RENDERER = TextContentRenderer.builder().build();
+    private static final TextContentRenderer SEPARATE_RENDERER = TextContentRenderer.builder()
+            .lineBreakRendering(LineBreakRendering.SEPARATE_BLOCKS).build();
+    private static final TextContentRenderer STRIPPED_RENDERER = TextContentRenderer.builder().stripNewlines(true).build();
+
     @Test
     public void textContentText() {
         String s;
@@ -179,37 +184,21 @@ public void textContentHtml() {
         assertAll(html, html);
     }
 
-    private TextContentRenderer compactRenderer() {
-        return TextContentRenderer.builder().build();
-    }
-
-    private TextContentRenderer separateBlocksRenderer() {
-        return TextContentRenderer.builder().lineBreakRendering(LineBreakRendering.SEPARATE_BLOCKS).build();
-    }
-
-    private TextContentRenderer strippedRenderer() {
-        return TextContentRenderer.builder().stripNewlines(true).build();
-    }
-
-    private Node parse(String source) {
-        return Parser.builder().build().parse(source);
-    }
-
     private void assertCompact(String source, String expected) {
-        var doc = parse(source);
-        var actualRendering = compactRenderer().render(doc);
+        var doc = PARSER.parse(source);
+        var actualRendering = COMPACT_RENDERER.render(doc);
         Asserts.assertRendering(source, expected, actualRendering);
     }
 
     private void assertSeparate(String source, String expected) {
-        var doc = parse(source);
-        var actualRendering = separateBlocksRenderer().render(doc);
+        var doc = PARSER.parse(source);
+        var actualRendering = SEPARATE_RENDERER.render(doc);
         Asserts.assertRendering(source, expected, actualRendering);
     }
 
     private void assertStripped(String source, String expected) {
-        var doc = parse(source);
-        var actualRendering = strippedRenderer().render(doc);
+        var doc = PARSER.parse(source);
+        var actualRendering = STRIPPED_RENDERER.render(doc);
         Asserts.assertRendering(source, expected, actualRendering);
     }