Refactor ContentTable builder and fix GFM table rendering (#1682)

2 files changed, 103 insertions, 23 deletions

diff --git a/plugins/gfm/src/main/kotlin/org/jetbrains/dokka/gfm/renderer/CommonmarkRenderer.kt b/plugins/gfm/src/main/kotlin/org/jetbrains/dokka/gfm/renderer/CommonmarkRenderer.kt
index 9fb92272..67552946 100644
--- a/plugins/gfm/src/main/kotlin/org/jetbrains/dokka/gfm/renderer/CommonmarkRenderer.kt
+++ b/plugins/gfm/src/main/kotlin/org/jetbrains/dokka/gfm/renderer/CommonmarkRenderer.kt
@@ -163,18 +163,18 @@ open class CommonmarkRenderer(
                 buildNewLine()
             }
         } else {
-            val size = node.header.size
+            val size = node.header.firstOrNull()?.children?.size ?: node.children.firstOrNull()?.children?.size ?: 0
 
             if (node.header.isNotEmpty()) {
-                append("| ")
                 node.header.forEach {
+                    append("| ")
                     it.children.forEach {
                         append(" ")
                         it.build(this, pageContext, it.sourceSets)
+                        append(" | ")
                     }
-                    append("| ")
+                    append("\n")
                 }
-                append("\n")
             } else {
                 append("| ".repeat(size))
                 if (size > 0) append("|\n")
@@ -196,7 +196,7 @@ open class CommonmarkRenderer(
                     )  // Workaround for headers inside tables
                 }
                 append(builder.toString().withEntersAsHtml())
-                append(" | ".repeat(size - it.children.size))
+                append("|".repeat(size + 1 - it.children.size))
                 append("\n")
             }
         }
@@ -339,4 +339,4 @@ open class CommonmarkRenderer(
 
     private fun StringBuilder.buildSourceSetTags(sourceSets: Set<DisplaySourceSet>) =
         append(sourceSets.joinToString(prefix = "[", postfix = "]") { it.name })
-}
\ No newline at end of file
+}
diff --git a/plugins/gfm/src/test/kotlin/renderers/gfm/SimpleElementsTest.kt b/plugins/gfm/src/test/kotlin/renderers/gfm/SimpleElementsTest.kt
index 7b887071..ef85d777 100644
--- a/plugins/gfm/src/test/kotlin/renderers/gfm/SimpleElementsTest.kt
+++ b/plugins/gfm/src/test/kotlin/renderers/gfm/SimpleElementsTest.kt
@@ -1,5 +1,6 @@
 package renderers.gfm
 
+import junit.framework.Assert.assertEquals
 import org.jetbrains.dokka.gfm.renderer.CommonmarkRenderer
 import org.junit.jupiter.api.Test
 import renderers.testPage
@@ -16,7 +17,7 @@ class SimpleElementsTest : GfmRenderingOnlyTestBase() {
         }
         val expect = "//[testPage](test-page.md)\n\n\n\n# The Hobbit or There and Back Again  \n"
         CommonmarkRenderer(context).render(page)
-        assert(renderedContent == expect)
+        assertEquals(expect, renderedContent)
     }
 
     @Test
@@ -24,19 +25,24 @@ class SimpleElementsTest : GfmRenderingOnlyTestBase() {
         val page = testPage {
             link("They are not all accounted for, the lost Seeing Stones.", "http://www.google.com")
         }
-        val expect = "//[testPage](test-page.md)\n\n[They are not all accounted for, the lost Seeing Stones.](http://www.google.com)"
+        val expect =
+            "//[testPage](test-page.md)\n\n[They are not all accounted for, the lost Seeing Stones.](http://www.google.com)"
         CommonmarkRenderer(context).render(page)
-        assert(renderedContent == expect)
+        assertEquals(expect, renderedContent)
     }
 
     @Test
     fun bold() {
         val page = testPage {
-            text("That there’s some good in this world, Mr. Frodo… and it’s worth fighting for.", styles = setOf(TextStyle.Bold))
+            text(
+                "That there’s some good in this world, Mr. Frodo… and it’s worth fighting for.",
+                styles = setOf(TextStyle.Bold)
+            )
         }
-        val expect = "//[testPage](test-page.md)\n\n**That there’s some good in this world, Mr. Frodo… and it’s worth fighting for.**"
+        val expect =
+            "//[testPage](test-page.md)\n\n**That there’s some good in this world, Mr. Frodo… and it’s worth fighting for.**"
         CommonmarkRenderer(context).render(page)
-        assert(renderedContent == expect)
+        assertEquals(expect, renderedContent)
     }
 
     @Test
@@ -46,31 +52,39 @@ class SimpleElementsTest : GfmRenderingOnlyTestBase() {
         }
         val expect = "//[testPage](test-page.md)\n\n*Even the smallest person can change the course of the future.*"
         CommonmarkRenderer(context).render(page)
-        assert(renderedContent == expect)
+        assertEquals(expect, renderedContent)
     }
 
     @Test
     fun italicAndBold() {
         val page = testPage {
-            text("There is no curse in Elvish, Entish, or the tongues of Men for this treachery.", styles = setOf(TextStyle.Bold, TextStyle.Italic))
+            text(
+                "There is no curse in Elvish, Entish, or the tongues of Men for this treachery.",
+                styles = setOf(TextStyle.Bold, TextStyle.Italic)
+            )
         }
-        val expect = "//[testPage](test-page.md)\n\n***There is no curse in Elvish, Entish, or the tongues of Men for this treachery.***"
+        val expect =
+            "//[testPage](test-page.md)\n\n***There is no curse in Elvish, Entish, or the tongues of Men for this treachery.***"
         CommonmarkRenderer(context).render(page)
-        assert(renderedContent == expect)
+        assertEquals(expect, renderedContent)
     }
 
     @Test
     fun strikethrough() {
         val page = testPage {
-            text("A day may come when the courage of men fails… but it is not THIS day", styles = setOf(TextStyle.Strikethrough))
+            text(
+                "A day may come when the courage of men fails… but it is not THIS day",
+                styles = setOf(TextStyle.Strikethrough)
+            )
         }
-        val expect = "//[testPage](test-page.md)\n\n~~A day may come when the courage of men fails… but it is not THIS day~~"
+        val expect =
+            "//[testPage](test-page.md)\n\n~~A day may come when the courage of men fails… but it is not THIS day~~"
         CommonmarkRenderer(context).render(page)
-        assert(renderedContent == expect)
+        assertEquals(expect, renderedContent)
     }
 
     @Test
-    fun images(){
+    fun images() {
         val image = ContentEmbeddedResource(
             children = emptyList(),
             address = "https://www.google.pl/images/branding/googlelogo/1x/googlelogo_color_272x92dp.png",
@@ -79,8 +93,74 @@ class SimpleElementsTest : GfmRenderingOnlyTestBase() {
             sourceSets = emptySet()
         )
         val page = RawTestPage(content = image)
-        val expect = "//[testPage](test-page.md)\n\n![This is a google logo](https://www.google.pl/images/branding/googlelogo/1x/googlelogo_color_272x92dp.png)"
+        val expect =
+            "//[testPage](test-page.md)\n\n![This is a google logo](https://www.google.pl/images/branding/googlelogo/1x/googlelogo_color_272x92dp.png)"
         CommonmarkRenderer(context).render(page)
-        assert(renderedContent == expect)
+        assertEquals(expect, renderedContent)
     }
-}
\ No newline at end of file
+
+    @Test
+    fun simpleTableWithHeader() {
+        val page = testPage {
+            table {
+                header {
+                    text("Col1")
+                    text("Col2")
+                    text("Col3")
+                }
+                row {
+                    text("Text1")
+                    text("Text2")
+                    text("Text3")
+                }
+                row {
+                    text("Text4")
+                    text("Text5")
+                    text("Text6")
+                }
+            }
+        }
+
+        val expect = """\//[testPage](test-page.md)
+            \
+            \  
+            \|  Col1 |  Col2 |  Col3 | 
+            \|---|---|---|
+            \| <a name="////PointingToDeclaration/"></a>Text1| <a name="////PointingToDeclaration/"></a>Text2| <a name="////PointingToDeclaration/"></a>Text3|
+            \| <a name="////PointingToDeclaration/"></a>Text4| <a name="////PointingToDeclaration/"></a>Text5| <a name="////PointingToDeclaration/"></a>Text6|
+            \""".trimMargin("\\")
+
+        CommonmarkRenderer(context).render(page)
+        assertEquals(expect, renderedContent)
+    }
+
+    @Test
+    fun simpleTableWithoutHeader() {
+        val page = testPage {
+            table {
+                row {
+                    text("Text1")
+                    text("Text2")
+                    text("Text3")
+                }
+                row {
+                    text("Text4")
+                    text("Text5")
+                    text("Text6")
+                }
+            }
+        }
+
+        val expect = """\//[testPage](test-page.md)
+            \
+            \  
+            \| | | |
+            \|---|---|---|
+            \| <a name="////PointingToDeclaration/"></a>Text1| <a name="////PointingToDeclaration/"></a>Text2| <a name="////PointingToDeclaration/"></a>Text3|
+            \| <a name="////PointingToDeclaration/"></a>Text4| <a name="////PointingToDeclaration/"></a>Text5| <a name="////PointingToDeclaration/"></a>Text6|
+            \""".trimMargin("\\")
+
+        CommonmarkRenderer(context).render(page)
+        assertEquals(expect, renderedContent)
+    }
+}