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

Sort groups of divergent elements by their key first ignoring case, then preserving it #2861

Merged
merged 2 commits into from Feb 17, 2023
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
Expand Up @@ -6,6 +6,7 @@ import org.jetbrains.dokka.base.transformers.documentables.isDeprecated
import org.jetbrains.dokka.base.transformers.documentables.isException
import org.jetbrains.dokka.base.translators.documentables.DocumentableLanguage
import org.jetbrains.dokka.base.translators.documentables.documentableLanguage
import org.jetbrains.dokka.base.utils.canonicalAlphabeticalOrder
import org.jetbrains.dokka.model.*
import org.jetbrains.dokka.pages.*

Expand Down Expand Up @@ -90,14 +91,17 @@ abstract class NavigationDataProvider {
}
}

private val navigationNodeOrder: Comparator<NavigationNode> =
compareBy(canonicalAlphabeticalOrder) { it.name }

private fun ContentPage.navigableChildren() =
if (this is ClasslikePage) {
this.navigableChildren()
} else {
children
.filterIsInstance<ContentPage>()
.map { visit(it) }
.sortedBy { it.name.toLowerCase() }
.sortedWith(navigationNodeOrder)
IgnatBeresnev marked this conversation as resolved.
Show resolved Hide resolved
}

private fun ClasslikePage.navigableChildren(): List<NavigationNode> {
Expand All @@ -111,7 +115,7 @@ abstract class NavigationDataProvider {
// no sorting for enum entries, should be the same order as in source code
navigableChildren
} else {
navigableChildren.sortedBy { it.name.toLowerCase() }
navigableChildren.sortedWith(navigationNodeOrder)
}
}
}
Expand Up @@ -9,6 +9,7 @@ import org.jetbrains.dokka.base.transformers.documentables.ClashingDriIdentifier
import org.jetbrains.dokka.base.transformers.pages.comments.CommentsToContentConverter
import org.jetbrains.dokka.base.transformers.pages.tags.CustomTagContentProvider
import org.jetbrains.dokka.base.translators.documentables.PageContentBuilder.DocumentableContentBuilder
import org.jetbrains.dokka.base.utils.canonicalAlphabeticalOrder
import org.jetbrains.dokka.links.Callable
import org.jetbrains.dokka.links.DRI
import org.jetbrains.dokka.model.*
Expand Down Expand Up @@ -508,6 +509,9 @@ open class DefaultPageCreator(
}
}

private val groupKeyComparator: Comparator<Map.Entry<String?, *>> =
compareBy(nullsFirst(canonicalAlphabeticalOrder)) { it.key }

protected open fun DocumentableContentBuilder.divergentBlock(
name: String,
collection: Collection<Documentable>,
Expand All @@ -525,7 +529,7 @@ open class DefaultPageCreator(
.groupBy { it.name } // This groupBy should probably use LocationProvider
// This hacks displaying actual typealias signatures along classlike ones
.mapValues { if (it.value.any { it is DClasslike }) it.value.filter { it !is DTypeAlias } else it.value }
.entries.sortedBy { it.key }
.entries.sortedWith(groupKeyComparator)
IgnatBeresnev marked this conversation as resolved.
Show resolved Hide resolved
.forEach { (elementName, elements) -> // This groupBy should probably use LocationProvider
val sortedElements = sortDivergentElementsDeterministically(elements)
row(
Expand Down
7 changes: 7 additions & 0 deletions plugins/base/src/main/kotlin/utils/alphabeticalOrder.kt
@@ -0,0 +1,7 @@
package org.jetbrains.dokka.base.utils


/**
* Canonical alphabetical order to sort named elements
*/
internal val canonicalAlphabeticalOrder: Comparator<in String> = String.CASE_INSENSITIVE_ORDER.thenBy { it }
40 changes: 40 additions & 0 deletions plugins/base/src/test/kotlin/pageMerger/PageNodeMergerTest.kt
Expand Up @@ -381,6 +381,46 @@ class PageNodeMergerTest : BaseAbstractTest() {
}
}

@Test
fun `should sort groups alphabetically ignoring case`() {
testInline(
"""
|/src/main/kotlin/test/Test.kt
|package test
|
|/** Sequence builder */
|fun <T> sequence(): Sequence<T>
|
|/** Sequence SAM constructor */
|fun <T> Sequence(): Sequence<T>
|
|/** Sequence.any() */
|fun <T> Sequence<T>.any() {}
|
|/** Sequence interface */
|interface Sequence<T>
""".trimMargin(),
defaultConfiguration
) {
renderingStage = { rootPageNode, _ ->
val packageFunctionBlocks = rootPageNode.findPackageFunctionBlocks(packageName = "test")
assertEquals(3, packageFunctionBlocks.size, "Expected 3 separate function groups")

packageFunctionBlocks[0].assertContainsKDocsInOrder(
"Sequence.any()",
)

packageFunctionBlocks[1].assertContainsKDocsInOrder(
"Sequence SAM constructor",
)

packageFunctionBlocks[2].assertContainsKDocsInOrder(
"Sequence builder",
)
}
}
}

private fun RootPageNode.findExtensionsOfClass(name: String): ContentDivergentGroup {
val extensionReceiverPage = this.dfs { it is ClasslikePageNode && it.name == name } as ClasslikePageNode
return extensionReceiverPage.content
Expand Down
74 changes: 74 additions & 0 deletions plugins/base/src/test/kotlin/renderers/html/NavigationTest.kt
Expand Up @@ -19,6 +19,80 @@ class NavigationTest : BaseAbstractTest() {
}
}

@Test
fun `should sort alphabetically ignoring case`() {
val writerPlugin = TestOutputWriterPlugin()
testInline(
"""
|/src/main/kotlin/com/example/Sequences.kt
|package com.example
|
|fun <T> sequence(): Sequence<T>
|
|fun <T> Sequence(): Sequence<T>
|
|fun <T> Sequence<T>.any() {}
|
|interface Sequence<T>
""".trimMargin(),
configuration,
pluginOverrides = listOf(writerPlugin)
) {
renderingStage = { _, _ ->
val content = writerPlugin.writer.navigationHtml().select("div.sideMenuPart")
assertEquals(6, content.size)

// Navigation menu should be the following:
// - root
// - com.example
// - any()
// - Sequence interface
// - Sequence()
// - sequence()

content[0].assertNavigationLink(
id = "root-nav-submenu",
text = "root",
address = "index.html",
)

content[1].assertNavigationLink(
id = "root-nav-submenu-0",
text = "com.example",
address = "root/com.example/index.html",
)

content[2].assertNavigationLink(
id = "root-nav-submenu-0-0",
text = "any()",
address = "root/com.example/any.html",
icon = NavigationNodeIcon.FUNCTION
)

content[3].assertNavigationLink(
id = "root-nav-submenu-0-1",
text = "Sequence",
address = "root/com.example/-sequence/index.html",
icon = NavigationNodeIcon.INTERFACE_KT
)

content[4].assertNavigationLink(
id = "root-nav-submenu-0-2",
text = "Sequence()",
address = "root/com.example/-sequence.html",
icon = NavigationNodeIcon.FUNCTION
)

content[5].assertNavigationLink(
id = "root-nav-submenu-0-3",
text = "sequence()",
address = "root/com.example/sequence.html",
icon = NavigationNodeIcon.FUNCTION
)
}
}
}

@Test
fun `should strike deprecated class link`() {
val writerPlugin = TestOutputWriterPlugin()
Expand Down