Skip to content

Commit

Permalink
Consistent alphabetical order of element groups in index and navigati…
Browse files Browse the repository at this point in the history
…on (#2861)

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

* Add tests for sorting groups and navigation
  • Loading branch information
ilya-g committed Feb 17, 2023
1 parent daed35f commit 1fd0459
Show file tree
Hide file tree
Showing 5 changed files with 132 additions and 3 deletions.
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)
}

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)
.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

0 comments on commit 1fd0459

Please sign in to comment.