From 63bed7cb2e47eb46772e680982c33ed3ee624c31 Mon Sep 17 00:00:00 2001 From: Ignat Beresnev Date: Wed, 31 May 2023 13:53:07 +0200 Subject: [PATCH] Fix missing tab entries for module names with space (#3019) --- .../kotlin/renderers/html/HtmlRenderer.kt | 20 +++--- .../dokka/scripts/platform-content-handler.js | 4 +- .../renderers/html/SourceSetFilterTest.kt | 64 +++++++++++++++++++ .../dokka/SourceSetArgumentsParser.kt | 6 ++ .../maven-plugin/src/main/kotlin/DokkaMojo.kt | 6 ++ 5 files changed, 89 insertions(+), 11 deletions(-) create mode 100644 plugins/base/src/test/kotlin/renderers/html/SourceSetFilterTest.kt diff --git a/plugins/base/src/main/kotlin/renderers/html/HtmlRenderer.kt b/plugins/base/src/main/kotlin/renderers/html/HtmlRenderer.kt index 768b2c6b06..94bd0aeb64 100644 --- a/plugins/base/src/main/kotlin/renderers/html/HtmlRenderer.kt +++ b/plugins/base/src/main/kotlin/renderers/html/HtmlRenderer.kt @@ -215,7 +215,7 @@ open class HtmlRenderer( node.isAnchorable -> buildAnchor( node.anchor!!, node.anchorLabel!!, - node.sourceSetsFilters + node.buildSourceSetFilterValues() ) { childrenCallback() } node.extra[InsertTemplateExtra] != null -> node.extra[InsertTemplateExtra]?.let { templateCommand(it.command) } ?: Unit @@ -568,10 +568,15 @@ open class HtmlRenderer( private fun FlowContent.addSourceSetFilteringAttributes( contextNode: ContentGroup, ) { - attributes["data-filterable-current"] = contextNode.sourceSets.joinToString(" ") { - it.sourceSetIDs.merged.toString() - } - attributes["data-filterable-set"] = contextNode.sourceSets.joinToString(" ") { + attributes["data-filterable-current"] = contextNode.buildSourceSetFilterValues() + attributes["data-filterable-set"] = contextNode.buildSourceSetFilterValues() + } + + private fun ContentNode.buildSourceSetFilterValues(): String { + // This value is used in HTML and JS for filtering out source set declarations, + // it is expected that the separator is the same here and there. + // See https://github.com/Kotlin/dokka/issues/3011#issuecomment-1568620493 + return this.sourceSets.joinToString(",") { it.sourceSetIDs.merged.toString() } } @@ -699,7 +704,7 @@ open class HtmlRenderer( buildAnchor(anchor, anchorLabel, sourceSets) {} private fun FlowContent.buildAnchor(node: ContentNode) { - node.anchorLabel?.let { label -> buildAnchor(node.anchor!!, label, node.sourceSetsFilters) } + node.anchorLabel?.let { label -> buildAnchor(node.anchor!!, label, node.buildSourceSetFilterValues()) } } @@ -982,8 +987,5 @@ private val PageNode.isNavigable: Boolean private fun PropertyContainer.extraHtmlAttributes() = allOfType() private fun PropertyContainer.extraTabbedContentType() = this[TabbedContentTypeExtra] -private val ContentNode.sourceSetsFilters: String - get() = sourceSets.sourceSetIDs.joinToString(" ") { it.toString() } - private val DisplaySourceSet.comparableKey get() = sourceSetIDs.merged.let { it.scopeId + it.sourceSetName } diff --git a/plugins/base/src/main/resources/dokka/scripts/platform-content-handler.js b/plugins/base/src/main/resources/dokka/scripts/platform-content-handler.js index 88d4d71a47..7c5e8af73d 100644 --- a/plugins/base/src/main/resources/dokka/scripts/platform-content-handler.js +++ b/plugins/base/src/main/resources/dokka/scripts/platform-content-handler.js @@ -322,8 +322,8 @@ function refreshFiltering() { document.querySelectorAll("[data-filterable-set]") .forEach( elem => { - let platformList = elem.getAttribute("data-filterable-set").split(' ').filter(v => -1 !== sourcesetList.indexOf(v)) - elem.setAttribute("data-filterable-current", platformList.join(' ')) + let platformList = elem.getAttribute("data-filterable-set").split(',').filter(v => -1 !== sourcesetList.indexOf(v)) + elem.setAttribute("data-filterable-current", platformList.join(',')) } ) refreshFilterButtons() diff --git a/plugins/base/src/test/kotlin/renderers/html/SourceSetFilterTest.kt b/plugins/base/src/test/kotlin/renderers/html/SourceSetFilterTest.kt new file mode 100644 index 0000000000..e615553545 --- /dev/null +++ b/plugins/base/src/test/kotlin/renderers/html/SourceSetFilterTest.kt @@ -0,0 +1,64 @@ +package renderers.html + +import org.jetbrains.dokka.base.testApi.testRunner.BaseAbstractTest +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Test +import signatures.renderedContent +import utils.TestOutputWriterPlugin + +class SourceSetFilterTest : BaseAbstractTest() { + + @Test // see #3011 + fun `should separate multiple data-filterable attribute values with comma`() { + val configuration = dokkaConfiguration { + moduleName = "Dokka Module" + + sourceSets { + val common = sourceSet { + name = "common" + displayName = "common" + analysisPlatform = "common" + sourceRoots = listOf("src/commonMain/kotlin/testing/Test.kt") + } + sourceSet { + name = "jvm" + displayName = "jvm" + analysisPlatform = "jvm" + dependentSourceSets = setOf(common.value.sourceSetID) + sourceRoots = listOf("src/jvmMain/kotlin/testing/Test.kt") + } + } + } + + val source = """ + |/src/commonMain/kotlin/testing/Test.kt + |package testing + | + |expect open class Test + | + |/src/jvmMain/kotlin/testing/Test.kt + |package testing + | + |actual open class Test + """.trimIndent() + + val writerPlugin = TestOutputWriterPlugin() + testInline( + source, + configuration, + pluginOverrides = listOf(writerPlugin) + ) { + renderingStage = { _, _ -> + val packagePage = writerPlugin.writer.renderedContent("-dokka -module/testing/index.html") + + val testClassRow = packagePage + .select("div[data-togglable=TYPE]") + .select("div[class=table-row]") + .single() + + assertEquals("Dokka Module/common,Dokka Module/jvm", testClassRow.attr("data-filterable-current")) + assertEquals("Dokka Module/common,Dokka Module/jvm", testClassRow.attr("data-filterable-set")) + } + } + } +} diff --git a/runners/cli/src/main/kotlin/org/jetbrains/dokka/SourceSetArgumentsParser.kt b/runners/cli/src/main/kotlin/org/jetbrains/dokka/SourceSetArgumentsParser.kt index 49e8f2ae05..120bd29652 100644 --- a/runners/cli/src/main/kotlin/org/jetbrains/dokka/SourceSetArgumentsParser.kt +++ b/runners/cli/src/main/kotlin/org/jetbrains/dokka/SourceSetArgumentsParser.kt @@ -7,6 +7,12 @@ import kotlinx.cli.delimiter internal fun parseSourceSet(moduleName: String, args: Array): DokkaConfiguration.DokkaSourceSet { + if (moduleName.contains(',')) { + // To figure out why this is needed and if it is still relevant, see the comment here: + // https://github.com/Kotlin/dokka/issues/3011#issuecomment-1568620493 + throw IllegalArgumentException("Module name cannot contain commas as it is used internally as a delimiter.") + } + val parser = ArgParser("sourceSet", prefixStyle = ArgParser.OptionPrefixStyle.JVM) val sourceSetName by parser.option( diff --git a/runners/maven-plugin/src/main/kotlin/DokkaMojo.kt b/runners/maven-plugin/src/main/kotlin/DokkaMojo.kt index c82f1b5965..6429333238 100644 --- a/runners/maven-plugin/src/main/kotlin/DokkaMojo.kt +++ b/runners/maven-plugin/src/main/kotlin/DokkaMojo.kt @@ -353,6 +353,12 @@ abstract class AbstractDokkaMojo(private val defaultDokkaPlugins: List { val links = mutableSetOf() if (!config.noJdkLink)