Add a distinct divider between function/property overloads (#2585)

7 files changed, 75 insertions, 31 deletions

diff --git a/core/src/main/kotlin/pages/ContentNodes.kt b/core/src/main/kotlin/pages/ContentNodes.kt
index a09a2cb5..a0f2fd74 100644
--- a/core/src/main/kotlin/pages/ContentNodes.kt
+++ b/core/src/main/kotlin/pages/ContentNodes.kt
@@ -36,7 +36,6 @@ data class ContentText(
     override fun hasAnyContent(): Boolean = text.isNotBlank()
 }
 
-// TODO: Remove
 data class ContentBreakLine(
     override val sourceSets: Set<DisplaySourceSet>,
     override val dci: DCI = DCI(emptySet(), ContentKind.Empty),
diff --git a/plugins/base/api/base.api b/plugins/base/api/base.api
index 4e1185d2..273ac519 100644
--- a/plugins/base/api/base.api
+++ b/plugins/base/api/base.api
@@ -243,6 +243,7 @@ public abstract class org/jetbrains/dokka/base/renderers/DefaultRenderer : org/j
 	public fun buildHeader (Ljava/lang/Object;Lorg/jetbrains/dokka/pages/ContentHeader;Lorg/jetbrains/dokka/pages/ContentPage;Ljava/util/Set;)V
 	public static synthetic fun buildHeader$default (Lorg/jetbrains/dokka/base/renderers/DefaultRenderer;Ljava/lang/Object;Lorg/jetbrains/dokka/pages/ContentHeader;Lorg/jetbrains/dokka/pages/ContentPage;Ljava/util/Set;ILjava/lang/Object;)V
 	public abstract fun buildLineBreak (Ljava/lang/Object;)V
+	public fun buildLineBreak (Ljava/lang/Object;Lorg/jetbrains/dokka/pages/ContentBreakLine;Lorg/jetbrains/dokka/pages/ContentPage;)V
 	public abstract fun buildLink (Ljava/lang/Object;Ljava/lang/String;Lkotlin/jvm/functions/Function1;)V
 	public abstract fun buildList (Ljava/lang/Object;Lorg/jetbrains/dokka/pages/ContentList;Lorg/jetbrains/dokka/pages/ContentPage;Ljava/util/Set;)V
 	public static synthetic fun buildList$default (Lorg/jetbrains/dokka/base/renderers/DefaultRenderer;Ljava/lang/Object;Lorg/jetbrains/dokka/pages/ContentList;Lorg/jetbrains/dokka/pages/ContentPage;Ljava/util/Set;ILjava/lang/Object;)V
@@ -359,7 +360,9 @@ public class org/jetbrains/dokka/base/renderers/html/HtmlRenderer : org/jetbrain
 	public fun buildHeader (Lkotlinx/html/FlowContent;ILorg/jetbrains/dokka/pages/ContentHeader;Lkotlin/jvm/functions/Function1;)V
 	public fun buildHtml (Lorg/jetbrains/dokka/pages/PageNode;Ljava/util/List;Lkotlin/jvm/functions/Function1;)Ljava/lang/String;
 	public synthetic fun buildLineBreak (Ljava/lang/Object;)V
+	public synthetic fun buildLineBreak (Ljava/lang/Object;Lorg/jetbrains/dokka/pages/ContentBreakLine;Lorg/jetbrains/dokka/pages/ContentPage;)V
 	public fun buildLineBreak (Lkotlinx/html/FlowContent;)V
+	public fun buildLineBreak (Lkotlinx/html/FlowContent;Lorg/jetbrains/dokka/pages/ContentBreakLine;Lorg/jetbrains/dokka/pages/ContentPage;)V
 	public synthetic fun buildLink (Ljava/lang/Object;Ljava/lang/String;Lkotlin/jvm/functions/Function1;)V
 	public fun buildLink (Lkotlinx/html/FlowContent;Ljava/lang/String;Lkotlin/jvm/functions/Function1;)V
 	public final fun buildLink (Lkotlinx/html/FlowContent;Lorg/jetbrains/dokka/links/DRI;Ljava/util/List;Lorg/jetbrains/dokka/pages/PageNode;Lkotlin/jvm/functions/Function1;)V
diff --git a/plugins/base/src/main/kotlin/renderers/DefaultRenderer.kt b/plugins/base/src/main/kotlin/renderers/DefaultRenderer.kt
index 2a21337f..d773650c 100644
--- a/plugins/base/src/main/kotlin/renderers/DefaultRenderer.kt
+++ b/plugins/base/src/main/kotlin/renderers/DefaultRenderer.kt
@@ -36,6 +36,8 @@ abstract class DefaultRenderer<T>(
     )
 
     abstract fun T.buildLineBreak()
+    open fun T.buildLineBreak(node: ContentBreakLine, pageContext: ContentPage) = buildLineBreak()
+
     abstract fun T.buildResource(node: ContentEmbeddedResource, pageContext: ContentPage)
     abstract fun T.buildTable(
         node: ContentTable,
@@ -116,7 +118,7 @@ abstract class DefaultRenderer<T>(
                 is ContentList -> buildList(node, pageContext, sourceSetRestriction)
                 is ContentTable -> buildTable(node, pageContext, sourceSetRestriction)
                 is ContentGroup -> buildGroup(node, pageContext, sourceSetRestriction)
-                is ContentBreakLine -> buildLineBreak()
+                is ContentBreakLine -> buildLineBreak(node, pageContext)
                 is PlatformHintedContent -> buildPlatformDependent(node, pageContext, sourceSetRestriction)
                 is ContentDivergentGroup -> buildDivergent(node, pageContext)
                 is ContentDivergentInstance -> buildDivergentInstance(node, pageContext)
@@ -233,4 +235,4 @@ abstract class DefaultRenderer<T>(
 internal typealias SerializedBeforeAndAfter = Pair<String, String>
 internal typealias InstanceWithSource = Pair<ContentDivergentInstance, DisplaySourceSet>
 
-fun ContentPage.sourceSets() = this.content.sourceSets
\ No newline at end of file
+fun ContentPage.sourceSets() = this.content.sourceSets
diff --git a/plugins/base/src/main/kotlin/renderers/html/HtmlContent.kt b/plugins/base/src/main/kotlin/renderers/html/HtmlContent.kt
new file mode 100644
index 00000000..a55323f9
--- /dev/null
+++ b/plugins/base/src/main/kotlin/renderers/html/HtmlContent.kt
@@ -0,0 +1,14 @@
+package org.jetbrains.dokka.base.renderers.html
+
+import org.jetbrains.dokka.pages.ContentBreakLine
+import org.jetbrains.dokka.pages.Style
+
+
+/**
+ * Html-specific style that represents <hr> tag if used in conjunction with [ContentBreakLine]
+ */
+internal object HorizontalBreakLineStyle : Style {
+    // this exists as a simple internal solution to avoid introducing unnecessary public API on content level.
+    // If you have the need to implement proper horizontal divider (i.e to support `---` markdown element),
+    // consider removing this and providing proper API for all formats and levels
+}
diff --git a/plugins/base/src/main/kotlin/renderers/html/HtmlRenderer.kt b/plugins/base/src/main/kotlin/renderers/html/HtmlRenderer.kt
index 5d2007d6..53645f06 100644
--- a/plugins/base/src/main/kotlin/renderers/html/HtmlRenderer.kt
+++ b/plugins/base/src/main/kotlin/renderers/html/HtmlRenderer.kt
@@ -14,16 +14,13 @@ import org.jetbrains.dokka.base.resolvers.anchors.SymbolAnchorHint
 import org.jetbrains.dokka.base.resolvers.local.DokkaBaseLocationProvider
 import org.jetbrains.dokka.base.templating.*
 import org.jetbrains.dokka.links.DRI
-import org.jetbrains.dokka.model.DisplaySourceSet
+import org.jetbrains.dokka.model.*
 import org.jetbrains.dokka.model.properties.PropertyContainer
-import org.jetbrains.dokka.model.sourceSetIDs
-import org.jetbrains.dokka.model.withDescendants
 import org.jetbrains.dokka.pages.*
 import org.jetbrains.dokka.pages.HtmlContent
 import org.jetbrains.dokka.plugability.*
 import org.jetbrains.dokka.utilities.htmlEscape
 import org.jetbrains.kotlin.utils.addIfNotNull
-import java.net.URI
 
 internal const val TEMPLATE_REPLACEMENT: String = "###"
 
@@ -252,23 +249,6 @@ open class HtmlRenderer(
     }
 
     override fun FlowContent.buildDivergent(node: ContentDivergentGroup, pageContext: ContentPage) {
-        fun groupDivergentInstancesWithSourceSet(
-            instances: List<ContentDivergentInstance>,
-            sourceSet: DisplaySourceSet,
-            pageContext: ContentPage,
-            beforeTransformer: (ContentDivergentInstance, ContentPage, DisplaySourceSet) -> String,
-            afterTransformer: (ContentDivergentInstance, ContentPage, DisplaySourceSet) -> String
-        ): Map<SerializedBeforeAndAfter, List<ContentDivergentInstance>> =
-            instances.map { instance ->
-                instance to Pair(
-                    beforeTransformer(instance, pageContext, sourceSet),
-                    afterTransformer(instance, pageContext, sourceSet)
-                )
-            }.groupBy(
-                Pair<ContentDivergentInstance, SerializedBeforeAndAfter>::second,
-                Pair<ContentDivergentInstance, SerializedBeforeAndAfter>::first
-            )
-
         if (node.implicitlySourceSetHinted) {
             val groupedInstancesBySourceSet = node.children.flatMap { instance ->
                 instance.sourceSets.map { sourceSet -> instance to sourceSet }
@@ -294,16 +274,28 @@ open class HtmlRenderer(
                                 }
                             }.stripDiv()
                         })
+
+                val isPageWithOverloadedMembers = pageContext is MemberPage && pageContext.documentables().size > 1
+
                 val contentOfSourceSet = mutableListOf<ContentNode>()
-                distinct.onEachIndexed{ i, (_, distinctInstances) ->
+                distinct.onEachIndexed{ index, (_, distinctInstances) ->
                     contentOfSourceSet.addIfNotNull(distinctInstances.firstOrNull()?.before)
                     contentOfSourceSet.addAll(distinctInstances.map { it.divergent })
                     contentOfSourceSet.addIfNotNull(
                         distinctInstances.firstOrNull()?.after
-                            ?: if (i != distinct.size - 1) ContentBreakLine(it.key) else null
+                            ?: if (index != distinct.size - 1) ContentBreakLine(it.key) else null
                     )
-                    if(node.dci.kind == ContentKind.Main && i != distinct.size - 1)
-                        contentOfSourceSet.add(ContentBreakLine(it.key))
+
+                    // content kind main is important for declarations list to avoid double line breaks
+                    if (node.dci.kind == ContentKind.Main && index != distinct.size - 1) {
+                        if (isPageWithOverloadedMembers) {
+                            // add some spacing and distinction between function/property overloads.
+                            // not ideal, but there's no other place to modify overloads page atm
+                            contentOfSourceSet.add(ContentBreakLine(it.key, style = setOf(HorizontalBreakLineStyle)))
+                        } else {
+                            contentOfSourceSet.add(ContentBreakLine(it.key))
+                        }
+                    }
                 }
                 contentOfSourceSet
             }
@@ -315,6 +307,27 @@ open class HtmlRenderer(
         }
     }
 
+    private fun groupDivergentInstancesWithSourceSet(
+        instances: List<ContentDivergentInstance>,
+        sourceSet: DisplaySourceSet,
+        pageContext: ContentPage,
+        beforeTransformer: (ContentDivergentInstance, ContentPage, DisplaySourceSet) -> String,
+        afterTransformer: (ContentDivergentInstance, ContentPage, DisplaySourceSet) -> String
+    ): Map<SerializedBeforeAndAfter, List<ContentDivergentInstance>> =
+        instances.map { instance ->
+            instance to Pair(
+                beforeTransformer(instance, pageContext, sourceSet),
+                afterTransformer(instance, pageContext, sourceSet)
+            )
+        }.groupBy(
+            Pair<ContentDivergentInstance, SerializedBeforeAndAfter>::second,
+            Pair<ContentDivergentInstance, SerializedBeforeAndAfter>::first
+        )
+
+    private fun ContentPage.documentables(): List<Documentable> {
+        return (this as? WithDocumentables)?.documentables ?: emptyList()
+    }
+
     override fun FlowContent.buildList(
         node: ContentList,
         pageContext: ContentPage,
@@ -672,6 +685,13 @@ open class HtmlRenderer(
     override fun buildError(node: ContentNode) = context.logger.error("Unknown ContentNode type: $node")
 
     override fun FlowContent.buildLineBreak() = br()
+    override fun FlowContent.buildLineBreak(node: ContentBreakLine, pageContext: ContentPage) {
+        if (node.style.contains(HorizontalBreakLineStyle)) {
+            hr()
+        } else {
+            buildLineBreak()
+        }
+    }
 
     override fun FlowContent.buildLink(address: String, content: FlowContent.() -> Unit) =
         a(href = address, block = content)
@@ -772,9 +792,6 @@ open class HtmlRenderer(
             content(this, page)
         }
 
-    private val String.isAbsolute: Boolean
-        get() = URI(this).isAbsolute
-
 
     open fun buildHtml(page: PageNode, resources: List<String>, content: FlowContent.() -> Unit): String =
         templater.renderFromTemplate(DokkaTemplateTypes.BASE) {
diff --git a/plugins/base/src/main/kotlin/transformers/pages/merger/SameMethodNamePageMergerStrategy.kt b/plugins/base/src/main/kotlin/transformers/pages/merger/SameMethodNamePageMergerStrategy.kt
index 6c12c719..003d68cf 100644
--- a/plugins/base/src/main/kotlin/transformers/pages/merger/SameMethodNamePageMergerStrategy.kt
+++ b/plugins/base/src/main/kotlin/transformers/pages/merger/SameMethodNamePageMergerStrategy.kt
@@ -6,6 +6,10 @@ import org.jetbrains.dokka.model.dfs
 import org.jetbrains.dokka.pages.*
 import org.jetbrains.dokka.utilities.DokkaLogger
 
+/**
+ * Merges [MemberPage] elements that have the same name.
+ * That includes **both** properties and functions.
+ */
 class SameMethodNamePageMergerStrategy(val logger: DokkaLogger) : PageMergerStrategy {
     override fun tryMerge(pages: List<PageNode>, path: List<String>): List<PageNode> {
         val members = pages.filterIsInstance<MemberPageNode>().takeIf { it.isNotEmpty() } ?: return pages
diff --git a/plugins/base/src/main/resources/dokka/styles/style.css b/plugins/base/src/main/resources/dokka/styles/style.css
index 50125756..989b54b8 100644
--- a/plugins/base/src/main/resources/dokka/styles/style.css
+++ b/plugins/base/src/main/resources/dokka/styles/style.css
@@ -115,6 +115,11 @@ html ::-webkit-scrollbar-thumb {
     margin-right: var(--horizontal-spacing-for-content);
 }
 
+.main-content .content > hr {
+    margin: 30px 0;
+    border-top: 3px double #8c8b8b;
+}
+
 .navigation-wrapper {
     display: flex;
     flex-wrap: wrap;