Cleanup paragraphs and tables in GFM renderer (#1946)

* GFM renderer: cleanup paragraphs

* GFM renderer: cleanup tables

* GFM renderer: add tests for wrong header in table

Table with extra cell in row is really generated by `all-modules-page` plugin

* Remove commented-out lines

* Add BriefCommentPreprocessor which inserts a line break between a signature and a brief comment

Fixed a bug with `mapTransform` function which replaces table headers with their contents

Co-authored-by: Kamil Doległo <kamil.doleglo@interia.pl>

3 files changed, 53 insertions, 49 deletions

diff --git a/plugins/gfm/src/main/kotlin/org/jetbrains/dokka/gfm/GfmPlugin.kt b/plugins/gfm/src/main/kotlin/org/jetbrains/dokka/gfm/GfmPlugin.kt
index 4ca639b2..78597d44 100644
--- a/plugins/gfm/src/main/kotlin/org/jetbrains/dokka/gfm/GfmPlugin.kt
+++ b/plugins/gfm/src/main/kotlin/org/jetbrains/dokka/gfm/GfmPlugin.kt
@@ -6,6 +6,7 @@ import org.jetbrains.dokka.base.renderers.PackageListCreator
 import org.jetbrains.dokka.base.renderers.RootCreator
 import org.jetbrains.dokka.base.resolvers.shared.RecognizedLinkFormat
 import org.jetbrains.dokka.gfm.location.MarkdownLocationProvider
+import org.jetbrains.dokka.gfm.renderer.BriefCommentPreprocessor
 import org.jetbrains.dokka.gfm.renderer.CommonmarkRenderer
 import org.jetbrains.dokka.plugability.DokkaPlugin
 import org.jetbrains.dokka.transformers.pages.PageTransformer
@@ -28,6 +29,10 @@ class GfmPlugin : DokkaPlugin() {
         gfmPreprocessors with RootCreator
     }
 
+    val briefCommentPreprocessor by extending {
+        gfmPreprocessors with BriefCommentPreprocessor()
+    }
+
     val packageListCreator by extending {
         (gfmPreprocessors
                 providing { PackageListCreator(it, RecognizedLinkFormat.DokkaGFM) }
diff --git a/plugins/gfm/src/main/kotlin/org/jetbrains/dokka/gfm/renderer/BriefCommentPreprocessor.kt b/plugins/gfm/src/main/kotlin/org/jetbrains/dokka/gfm/renderer/BriefCommentPreprocessor.kt
new file mode 100644
index 00000000..fd65e2f9
--- /dev/null
+++ b/plugins/gfm/src/main/kotlin/org/jetbrains/dokka/gfm/renderer/BriefCommentPreprocessor.kt
@@ -0,0 +1,16 @@
+package org.jetbrains.dokka.gfm.renderer
+
+import org.jetbrains.dokka.pages.*
+import org.jetbrains.dokka.transformers.pages.PageTransformer
+
+class BriefCommentPreprocessor : PageTransformer {
+    override fun invoke(input: RootPageNode) = input.transformContentPagesTree { contentPage ->
+        contentPage.modified(content = contentPage.content.recursiveMapTransform<ContentGroup, ContentNode> {
+            if (it.dci.kind == ContentKind.BriefComment) {
+                it.copy(style = it.style + setOf(TextStyle.Block))
+            } else {
+                it
+            }
+        })
+    }
+}
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 676b0606..f18c7093 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
@@ -26,11 +26,7 @@ open class CommonmarkRenderer(
         childrenCallback: StringBuilder.() -> Unit
     ) {
         return when {
-            node.hasStyle(TextStyle.Block) -> {
-                childrenCallback()
-                buildParagraph()
-            }
-            node.hasStyle(TextStyle.Paragraph) -> {
+            node.hasStyle(TextStyle.Block) || node.hasStyle(TextStyle.Paragraph) -> {
                 buildParagraph()
                 childrenCallback()
                 buildParagraph()
@@ -43,7 +39,7 @@ open class CommonmarkRenderer(
         buildParagraph()
         append("#".repeat(level) + " ")
         content()
-        appendNewLine()
+        buildParagraph()
     }
 
     override fun StringBuilder.buildLink(address: String, content: StringBuilder.() -> Unit) {
@@ -57,7 +53,9 @@ open class CommonmarkRenderer(
         pageContext: ContentPage,
         sourceSetRestriction: Set<DisplaySourceSet>?
     ) {
+        buildParagraph()
         buildListLevel(node, pageContext)
+        buildParagraph()
     }
 
     private fun StringBuilder.buildListItem(items: List<ContentNode>, pageContext: ContentPage) {
@@ -135,10 +133,11 @@ open class CommonmarkRenderer(
             }.groupBy(Pair<DisplaySourceSet, String>::second, Pair<DisplaySourceSet, String>::first)
 
             distinct.filter { it.key.isNotBlank() }.forEach { (text, platforms) ->
-                append(" ")
+                buildParagraph()
                 buildSourceSetTags(platforms.toSet())
-                append(" $text")
-                appendNewLine()
+                buildNewLine()
+                append(text.trim())
+                buildParagraph()
             }
         }
     }
@@ -170,46 +169,40 @@ open class CommonmarkRenderer(
             }
         } else {
             val size = node.header.firstOrNull()?.children?.size ?: node.children.firstOrNull()?.children?.size ?: 0
+            if (size <= 0) return
 
             if (node.header.isNotEmpty()) {
                 node.header.forEach {
-                    append("| ")
                     it.children.forEach {
-                        append(" ")
+                        append("| ")
                         it.build(this, pageContext, it.sourceSets)
-                        append(" | ")
+                        append(" ")
                     }
-                    append("\n")
                 }
             } else {
                 append("| ".repeat(size))
-                if (size > 0) {
-                    append("|")
-                    appendNewLine()
-                }
             }
+            append("|")
+            appendNewLine()
 
             append("|---".repeat(size))
-            if (size > 0) {
-                append("|")
-                appendNewLine()
-            }
+            append("|")
+            appendNewLine()
 
-            node.children.forEach {
-                val builder = StringBuilder()
-                it.children.forEach {
-                    builder.append("| ")
-                    builder.append("<a name=\"${it.dci.dri.first()}\"></a>")
-                    builder.append(
-                        buildString { it.build(this, pageContext) }.replace(
-                            Regex("#+ "),
-                            ""
-                        )
-                    )  // Workaround for headers inside tables
+            node.children.forEach { row ->
+                row.children.forEach { cell ->
+                    append("| ")
+                    append(buildString { cell.build(this, pageContext) }
+                        .trim()
+                        .replace("#+ ".toRegex(), "") // Workaround for headers inside tables
+                        .replace("\\\n", "\n\n")
+                        .replace("\n[\n]+".toRegex(), "<br>")
+                        .replace("\n", " ")
+                    )
+                    append(" ")
                 }
-                append(builder.toString().withEntersAsHtml())
-                append("|".repeat(size + 1 - it.children.size))
-                append("\n")
+                append("|")
+                appendNewLine()
             }
         }
     }
@@ -259,25 +252,22 @@ open class CommonmarkRenderer(
         distinct.values.forEach { entry ->
             val (instance, sourceSets) = entry.getInstanceAndSourceSets()
 
+            buildParagraph()
             buildSourceSetTags(sourceSets)
+            buildNewLine()
 
             instance.before?.let {
-                buildNewLine()
-                append("Brief description")
-                buildNewLine()
                 buildContentNode(
                     it,
                     pageContext,
                     sourceSets.first()
                 ) // It's workaround to render content only once
+                buildParagraph()
             }
 
-            buildNewLine()
-            append("Content")
             entry.groupBy { buildString { buildContentNode(it.first.divergent, pageContext, setOf(it.second)) } }
                 .values.forEach { innerEntry ->
                     val (innerInstance, innerSourceSets) = innerEntry.getInstanceAndSourceSets()
-                    buildNewLine()
                     if (sourceSets.size > 1) {
                         buildSourceSetTags(innerSourceSets)
                         buildNewLine()
@@ -287,12 +277,10 @@ open class CommonmarkRenderer(
                         pageContext,
                         setOf(innerSourceSets.first())
                     ) // It's workaround to render content only once
+                    buildParagraph()
                 }
 
             instance.after?.let {
-                buildNewLine()
-                append("More info")
-                buildNewLine()
                 buildContentNode(
                     it,
                     pageContext,
@@ -358,11 +346,6 @@ open class CommonmarkRenderer(
         }
     }
 
-    private fun String.withEntersAsHtml(): String = this
-        .replace("\\\n", "\n\n")
-        .replace("\n[\n]+".toRegex(), "<br>")
-        .replace("\n", " ")
-
     private fun List<Pair<ContentDivergentInstance, DisplaySourceSet>>.getInstanceAndSourceSets() =
         this.let { Pair(it.first().first, it.map { it.second }.toSet()) }