Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a distinct divider between function overloads #2585

Merged
merged 1 commit into from Aug 5, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 0 additions & 1 deletion core/src/main/kotlin/pages/ContentNodes.kt
Expand Up @@ -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),
Expand Down
3 changes: 3 additions & 0 deletions plugins/base/api/base.api
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
6 changes: 4 additions & 2 deletions plugins/base/src/main/kotlin/renderers/DefaultRenderer.kt
Expand Up @@ -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,
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
fun ContentPage.sourceSets() = this.content.sourceSets
14 changes: 14 additions & 0 deletions 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
}
73 changes: 45 additions & 28 deletions plugins/base/src/main/kotlin/renderers/html/HtmlRenderer.kt
Expand Up @@ -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 = "###"

Expand Down Expand Up @@ -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 }
Expand All @@ -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
}
Expand All @@ -315,6 +307,27 @@ open class HtmlRenderer(
}
}

private fun groupDivergentInstancesWithSourceSet(
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This made buildDivergent considerably longer, even though this particular function needed no context from it and shared nothing in common, so I don't see a reason why it has to be a nested function.

Extracted for the sake of readability of buildDivergent

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,
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -772,9 +792,6 @@ open class HtmlRenderer(
content(this, page)
}

private val String.isAbsolute: Boolean
get() = URI(this).isAbsolute

Comment on lines -775 to -777
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused


open fun buildHtml(page: PageNode, resources: List<String>, content: FlowContent.() -> Unit): String =
templater.renderFromTemplate(DokkaTemplateTypes.BASE) {
Expand Down
Expand Up @@ -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
Expand Down
5 changes: 5 additions & 0 deletions plugins/base/src/main/resources/dokka/styles/style.css
Expand Up @@ -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;
Expand Down