From 3994c42f2f2366d2e551e62412518a210581cb3a Mon Sep 17 00:00:00 2001 From: Ignat Beresnev Date: Wed, 3 Aug 2022 13:51:50 +0200 Subject: [PATCH] Apply the same style to all KDoc tags, including throws/see/parameters (#2587) --- core/api/core.api | 1 + core/src/main/kotlin/pages/ContentNodes.kt | 2 +- .../kotlin/renderers/html/HtmlRenderer.kt | 1 + .../tags/SinceKotlinTagContentProvider.kt | 8 +- .../documentables/DefaultPageCreator.kt | 54 ++++++++----- .../src/main/resources/dokka/styles/style.css | 11 ++- .../content/params/ContentForParamsTest.kt | 79 +++++++++++++++---- .../content/seealso/ContentForSeeAlsoTest.kt | 78 +++++++++++++++--- 8 files changed, 180 insertions(+), 54 deletions(-) diff --git a/core/api/core.api b/core/api/core.api index 9f290fbd50..0487146d76 100644 --- a/core/api/core.api +++ b/core/api/core.api @@ -3912,6 +3912,7 @@ public final class org/jetbrains/dokka/pages/ContentStyle : java/lang/Enum, org/ public static final field Caption Lorg/jetbrains/dokka/pages/ContentStyle; public static final field InDocumentationAnchor Lorg/jetbrains/dokka/pages/ContentStyle; public static final field Indented Lorg/jetbrains/dokka/pages/ContentStyle; + public static final field KDocTag Lorg/jetbrains/dokka/pages/ContentStyle; public static final field RowTitle Lorg/jetbrains/dokka/pages/ContentStyle; public static final field RunnableSample Lorg/jetbrains/dokka/pages/ContentStyle; public static final field TabbedContent Lorg/jetbrains/dokka/pages/ContentStyle; diff --git a/core/src/main/kotlin/pages/ContentNodes.kt b/core/src/main/kotlin/pages/ContentNodes.kt index 07ff115903..a09a2cb501 100644 --- a/core/src/main/kotlin/pages/ContentNodes.kt +++ b/core/src/main/kotlin/pages/ContentNodes.kt @@ -389,7 +389,7 @@ enum class TextStyle : Style { enum class ContentStyle : Style { RowTitle, TabbedContent, WithExtraAttributes, RunnableSample, InDocumentationAnchor, Caption, - Wrapped, Indented + Wrapped, Indented, KDocTag } enum class ListStyle : Style { diff --git a/plugins/base/src/main/kotlin/renderers/html/HtmlRenderer.kt b/plugins/base/src/main/kotlin/renderers/html/HtmlRenderer.kt index a28fcd0477..b586b95ed5 100644 --- a/plugins/base/src/main/kotlin/renderers/html/HtmlRenderer.kt +++ b/plugins/base/src/main/kotlin/renderers/html/HtmlRenderer.kt @@ -96,6 +96,7 @@ open class HtmlRenderer( node.dci.kind in setOf(ContentKind.Symbol) -> div("symbol $additionalClasses") { childrenCallback() } + node.hasStyle(ContentStyle.KDocTag) -> span("kdoc-tag") { childrenCallback() } node.hasStyle(TextStyle.BreakableAfter) -> { span { childrenCallback() } wbr { } diff --git a/plugins/base/src/main/kotlin/transformers/pages/tags/SinceKotlinTagContentProvider.kt b/plugins/base/src/main/kotlin/transformers/pages/tags/SinceKotlinTagContentProvider.kt index c9010421b9..a1d3090363 100644 --- a/plugins/base/src/main/kotlin/transformers/pages/tags/SinceKotlinTagContentProvider.kt +++ b/plugins/base/src/main/kotlin/transformers/pages/tags/SinceKotlinTagContentProvider.kt @@ -1,9 +1,9 @@ package org.jetbrains.dokka.base.transformers.pages.tags import org.jetbrains.dokka.DokkaConfiguration +import org.jetbrains.dokka.base.translators.documentables.KDOC_TAG_HEADER_LEVEL import org.jetbrains.dokka.base.translators.documentables.PageContentBuilder.DocumentableContentBuilder import org.jetbrains.dokka.model.doc.CustomTagWrapper -import org.jetbrains.dokka.pages.ContentKind import org.jetbrains.dokka.pages.TextStyle object SinceKotlinTagContentProvider : CustomTagContentProvider { @@ -16,8 +16,8 @@ object SinceKotlinTagContentProvider : CustomTagContentProvider { sourceSet: DokkaConfiguration.DokkaSourceSet, customTag: CustomTagWrapper ) { - group(sourceSets = setOf(sourceSet), kind = ContentKind.Comment, styles = setOf(TextStyle.Block)) { - header(4, customTag.name) + group(sourceSets = setOf(sourceSet), styles = emptySet()) { + header(KDOC_TAG_HEADER_LEVEL, customTag.name) comment(customTag.root) } } @@ -31,4 +31,4 @@ object SinceKotlinTagContentProvider : CustomTagContentProvider { comment(customTag.root, styles = emptySet()) } } -} \ No newline at end of file +} diff --git a/plugins/base/src/main/kotlin/translators/documentables/DefaultPageCreator.kt b/plugins/base/src/main/kotlin/translators/documentables/DefaultPageCreator.kt index a8b33d4cd7..f08b2056cc 100644 --- a/plugins/base/src/main/kotlin/translators/documentables/DefaultPageCreator.kt +++ b/plugins/base/src/main/kotlin/translators/documentables/DefaultPageCreator.kt @@ -21,6 +21,8 @@ import org.jetbrains.kotlin.utils.addToStdlib.safeAs import kotlin.reflect.KClass import kotlin.reflect.full.isSubclassOf +internal const val KDOC_TAG_HEADER_LEVEL = 4 + private typealias GroupedTags = Map, List>> private val specialTags: Set> = @@ -463,11 +465,11 @@ open class DefaultPageCreator( val customTags = d.customTags if (customTags.isNotEmpty()) { - group(styles = setOf(TextStyle.Block)) { - platforms.forEach { platform -> - customTags.forEach { (_, sourceSetTag) -> - sourceSetTag[platform]?.let { tag -> - customTagContentProviders.filter { it.isApplicable(tag) }.forEach { provider -> + platforms.forEach { platform -> + customTags.forEach { (_, sourceSetTag) -> + sourceSetTag[platform]?.let { tag -> + customTagContentProviders.filter { it.isApplicable(tag) }.forEach { provider -> + group(sourceSets = setOf(platform), styles = setOf(ContentStyle.KDocTag)) { with(provider) { contentForDescription(platform, tag) } @@ -485,9 +487,13 @@ open class DefaultPageCreator( unnamedTags[platform]?.let { tags -> if (tags.isNotEmpty()) { tags.groupBy { it::class }.forEach { (_, sameCategoryTags) -> - group(sourceSets = setOf(platform), styles = emptySet()) { - header(4, sameCategoryTags.first().toHeaderString()) - sameCategoryTags.forEach { comment(it.root) } + group(sourceSets = setOf(platform), styles = setOf(ContentStyle.KDocTag)) { + header( + level = KDOC_TAG_HEADER_LEVEL, + text = sameCategoryTags.first().toHeaderString(), + styles = setOf() + ) + sameCategoryTags.forEach { comment(it.root, styles = setOf()) } } } } @@ -537,7 +543,7 @@ open class DefaultPageCreator( val params = tags.withTypeNamed() val availablePlatforms = params.values.flatMap { it.keys }.toSet() - header(2, "Parameters", kind = ContentKind.Parameters, sourceSets = availablePlatforms) + header(KDOC_TAG_HEADER_LEVEL, "Parameters", kind = ContentKind.Parameters, sourceSets = availablePlatforms) group( extra = mainExtra + SimpleAttr.header("Parameters"), styles = setOf(ContentStyle.WithExtraAttributes), @@ -555,7 +561,9 @@ open class DefaultPageCreator( kind = ContentKind.Parameters, styles = mainStyles + ContentStyle.RowTitle ) - comment(it.root) + if (it.isNotEmpty()) { + comment(it.root) + } } } } @@ -571,7 +579,7 @@ open class DefaultPageCreator( val seeAlsoTags = tags.withTypeNamed() val availablePlatforms = seeAlsoTags.values.flatMap { it.keys }.toSet() - header(2, "See also", kind = ContentKind.Comment, sourceSets = availablePlatforms) + header(KDOC_TAG_HEADER_LEVEL, "See also", kind = ContentKind.Comment, sourceSets = availablePlatforms) group( extra = mainExtra + SimpleAttr.header("See also"), styles = setOf(ContentStyle.WithExtraAttributes), @@ -590,7 +598,7 @@ open class DefaultPageCreator( ) { it.address?.let { dri -> link( - it.name, + dri.classNames ?: it.name, dri, kind = ContentKind.Comment, styles = mainStyles + ContentStyle.RowTitle @@ -600,7 +608,9 @@ open class DefaultPageCreator( kind = ContentKind.Comment, styles = mainStyles + ContentStyle.RowTitle ) - comment(it.root) + if (it.isNotEmpty()) { + comment(it.root) + } } } } @@ -616,19 +626,25 @@ open class DefaultPageCreator( if (throws.isNotEmpty()) { val availablePlatforms = throws.values.flatMap { it.keys }.toSet() - header(2, "Throws", sourceSets = availablePlatforms) + header(KDOC_TAG_HEADER_LEVEL, "Throws", sourceSets = availablePlatforms) buildContent(availablePlatforms) { availablePlatforms.forEach { sourceset -> - table(kind = ContentKind.Main, sourceSets = setOf(sourceset)) { + table( + kind = ContentKind.Main, + sourceSets = setOf(sourceset), + extra = mainExtra + SimpleAttr.header("Throws") + ) { throws.entries.forEach { entry -> entry.value[sourceset]?.let { throws -> row(sourceSets = setOf(sourceset)) { group(styles = mainStyles + ContentStyle.RowTitle) { throws.exceptionAddress?.let { - link(text = entry.key, address = it) + link(text = it.classNames ?: entry.key, address = it) } ?: text(entry.key) } - comment(throws.root) + if (throws.isNotEmpty()) { + comment(throws.root) + } } } } @@ -642,7 +658,7 @@ open class DefaultPageCreator( val samples = tags.withTypeNamed() if (samples.isNotEmpty()) { val availablePlatforms = samples.values.flatMap { it.keys }.toSet() - header(2, "Samples", kind = ContentKind.Sample, sourceSets = availablePlatforms) + header(KDOC_TAG_HEADER_LEVEL, "Samples", kind = ContentKind.Sample, sourceSets = availablePlatforms) group( extra = mainExtra + SimpleAttr.header("Samples"), styles = emptySet(), @@ -676,6 +692,8 @@ open class DefaultPageCreator( }.children } + private fun TagWrapper.isNotEmpty() = this.children.isNotEmpty() + protected open fun DocumentableContentBuilder.contentForBrief(documentable: Documentable) { documentable.sourceSets.forEach { sourceSet -> documentable.documentation[sourceSet]?.let { diff --git a/plugins/base/src/main/resources/dokka/styles/style.css b/plugins/base/src/main/resources/dokka/styles/style.css index 5b1e95e6fd..7e9761a655 100644 --- a/plugins/base/src/main/resources/dokka/styles/style.css +++ b/plugins/base/src/main/resources/dokka/styles/style.css @@ -239,6 +239,14 @@ p.paragraph:first-child, margin-top: 0; } +.content .kdoc-tag > p.paragraph { + margin-top: 0; +} + +.content h4 { + margin-bottom: 0; +} + .divergent-group { background-color: var(--background-color); padding: 16px 0 8px 0; @@ -1016,11 +1024,12 @@ td.content { .keyValue { display: grid; + grid-gap: 8px; } @media print, screen and (min-width: 960px) { .keyValue { - grid-template-columns: 20% 80%; + grid-template-columns: 25% 75%; } .title-row { diff --git a/plugins/base/src/test/kotlin/content/params/ContentForParamsTest.kt b/plugins/base/src/test/kotlin/content/params/ContentForParamsTest.kt index 50580afc66..7e11dc8573 100644 --- a/plugins/base/src/test/kotlin/content/params/ContentForParamsTest.kt +++ b/plugins/base/src/test/kotlin/content/params/ContentForParamsTest.kt @@ -450,17 +450,17 @@ class ContentForParamsTest : BaseAbstractTest() { } after { group { pWrapped("a normal comment") } - header(2) { +"Throws" } + header(4) { +"Throws" } table { group { group { - link { +"java.lang.IllegalStateException" } + link { +"IllegalStateException" } } comment { +"if the Dialog has not yet been created (before onCreateDialog) or has been destroyed (after onDestroyView)." } } group { group { - link { +"java.lang.RuntimeException" } + link { +"RuntimeException" } } comment { +"when " @@ -507,7 +507,7 @@ class ContentForParamsTest : BaseAbstractTest() { } after { group { pWrapped("a normal comment") } - header(2) { +"Throws" } + header(4) { +"Throws" } table { group { group { @@ -518,7 +518,7 @@ class ContentForParamsTest : BaseAbstractTest() { (this as ContentDRILink).address.toString() ) } - +"java.lang.IllegalStateException" + +"IllegalStateException" } } comment { +"if the Dialog has not yet been created (before onCreateDialog) or has been destroyed (after onDestroyView)." } @@ -532,7 +532,7 @@ class ContentForParamsTest : BaseAbstractTest() { (this as ContentDRILink).address.toString() ) } - +"java.lang.RuntimeException" + +"RuntimeException" } } comment { @@ -550,6 +550,51 @@ class ContentForParamsTest : BaseAbstractTest() { } } + @Test + fun `should display fully qualified throws name for unresolved class`() { + testInline( + """ + |/src/main/kotlin/sample/sample.kt + |package sample; + | /** + | * a normal comment + | * + | * @throws com.example.UnknownException description for non-resolved + | */ + | fun sample(){ } + """.trimIndent(), testConfiguration + ) { + pagesTransformationStage = { module -> + val functionPage = + module.children.single { it.name == "sample" }.children.single { it.name == "sample" } as ContentPage + functionPage.content.assertNode { + group { + header(1) { +"sample" } + } + divergentGroup { + divergentInstance { + divergent { + skipAllNotMatching() //Signature + } + after { + group { pWrapped("a normal comment") } + header(4) { +"Throws" } + table { + group { + group { + +"com.example.UnknownException" + } + comment { +"description for non-resolved" } + } + } + } + } + } + } + } + } + } + @Test fun `multiline throws where exception is not in the same line as description`() { testInline( @@ -585,7 +630,7 @@ class ContentForParamsTest : BaseAbstractTest() { } after { group { pWrapped("a normal comment") } - header(2) { +"Throws" } + header(4) { +"Throws" } table { group { group { @@ -596,7 +641,7 @@ class ContentForParamsTest : BaseAbstractTest() { (this as ContentDRILink).address.toString() ) } - +"java.lang.IllegalStateException" + +"IllegalStateException" } } comment { +"if the Dialog has not yet been created (before onCreateDialog) or has been destroyed (after onDestroyView)." } @@ -610,7 +655,7 @@ class ContentForParamsTest : BaseAbstractTest() { (this as ContentDRILink).address.toString() ) } - +"java.lang.RuntimeException" + +"RuntimeException" } } comment { @@ -719,7 +764,7 @@ class ContentForParamsTest : BaseAbstractTest() { +" doesn't contain value." } } - header(2) { +"Parameters" } + header(4) { +"Parameters" } group { table { group { @@ -1007,7 +1052,7 @@ class ContentForParamsTest : BaseAbstractTest() { } after { group { pWrapped("comment to function") } - header(2) { +"Parameters" } + header(4) { +"Parameters" } group { table { group { @@ -1060,7 +1105,7 @@ class ContentForParamsTest : BaseAbstractTest() { } after { group { group { group { +"comment to function" } } } - header(2) { +"Parameters" } + header(4) { +"Parameters" } group { table { group { @@ -1122,7 +1167,7 @@ class ContentForParamsTest : BaseAbstractTest() { } after { group { group { group { +"comment to function" } } } - header(2) { +"Parameters" } + header(4) { +"Parameters" } group { table { group { @@ -1181,7 +1226,7 @@ class ContentForParamsTest : BaseAbstractTest() { ) } after { - header(2) { +"Parameters" } + header(4) { +"Parameters" } group { table { group { @@ -1249,7 +1294,7 @@ class ContentForParamsTest : BaseAbstractTest() { header(4) { +"Receiver" } pWrapped("comment to receiver") } - header(2) { +"Parameters" } + header(4) { +"Parameters" } group { table { group { @@ -1301,7 +1346,7 @@ class ContentForParamsTest : BaseAbstractTest() { } after { group { group { group { +"comment to function" } } } - header(2) { +"Parameters" } + header(4) { +"Parameters" } group { table { group { @@ -1362,7 +1407,7 @@ class ContentForParamsTest : BaseAbstractTest() { group { pWrapped("comment to function") } unnamedTag("Author") { comment { +"Kordyjan" } } unnamedTag("Since") { comment { +"0.11" } } - header(2) { +"Parameters" } + header(4) { +"Parameters" } group { table { diff --git a/plugins/base/src/test/kotlin/content/seealso/ContentForSeeAlsoTest.kt b/plugins/base/src/test/kotlin/content/seealso/ContentForSeeAlsoTest.kt index 4096640a14..5c1fd05415 100644 --- a/plugins/base/src/test/kotlin/content/seealso/ContentForSeeAlsoTest.kt +++ b/plugins/base/src/test/kotlin/content/seealso/ContentForSeeAlsoTest.kt @@ -97,13 +97,12 @@ class ContentForSeeAlsoTest : BaseAbstractTest() { ) } after { - header(2) { +"See also" } + header(4) { +"See also" } group { table { group { //DRI should be "test//abc/#/-1/" link { +"abc" } - group { } } } } @@ -150,7 +149,7 @@ class ContentForSeeAlsoTest : BaseAbstractTest() { ) } after { - header(2) { +"See also" } + header(4) { +"See also" } group { table { group { @@ -170,6 +169,60 @@ class ContentForSeeAlsoTest : BaseAbstractTest() { } } + @Test + fun `should use fully qualified name for unresolved link`() { + testInline( + """ + |/src/main/kotlin/test/source.kt + |package test + | /** + | * @see com.example.NonExistingClass description for non-existing + | */ + |fun function(abc: String) { + | println(abc) + |} + """.trimIndent(), testConfiguration + ) { + pagesTransformationStage = { module -> + val page = module.children.single { it.name == "test" } + .children.single { it.name == "function" } as ContentPage + page.content.assertNode { + group { + header(1) { +"function" } + } + divergentGroup { + divergentInstance { + divergent { + bareSignature( + emptyMap(), + "", + "", + emptySet(), + "function", + null, + "abc" to ParamAttributes(emptyMap(), emptySet(), "String") + ) + } + after { + header(4) { +"See also" } + group { + table { + group { + +"com.example.NonExistingClass" + group { + group { +"description for non-existing" } + } + } + } + } + } + } + } + } + } + } + } + @Test fun `undocumented seealso with stdlib link`() { testInline( @@ -205,7 +258,7 @@ class ContentForSeeAlsoTest : BaseAbstractTest() { ) } after { - header(2) { +"See also" } + header(4) { +"See also" } group { table { group { @@ -216,9 +269,8 @@ class ContentForSeeAlsoTest : BaseAbstractTest() { (this as ContentDRILink).address.toString() ) } - +"kotlin.collections.Collection" + +"Collection" } - group { } } } } @@ -265,12 +317,12 @@ class ContentForSeeAlsoTest : BaseAbstractTest() { ) } after { - header(2) { +"See also" } + header(4) { +"See also" } group { table { group { //DRI should be "test//abc/#/-1/" - link { +"kotlin.collections.Collection" } + link { +"Collection" } group { group { +"Comment to stdliblink" } } @@ -327,12 +379,12 @@ class ContentForSeeAlsoTest : BaseAbstractTest() { unnamedTag("Author") { comment { +"pikinier20" } } unnamedTag("Since") { comment { +"0.11" } } - header(2) { +"See also" } + header(4) { +"See also" } group { table { group { //DRI should be "test//abc/#/-1/" - link { +"kotlin.collections.Collection" } + link { +"Collection" } group { group { +"Comment to stdliblink" } } @@ -383,7 +435,7 @@ class ContentForSeeAlsoTest : BaseAbstractTest() { ) } after { - header(2) { +"See also" } + header(4) { +"See also" } group { table { group { @@ -439,7 +491,7 @@ class ContentForSeeAlsoTest : BaseAbstractTest() { ) } after { - header(2) { +"See also" } + header(4) { +"See also" } group { table { group { @@ -451,7 +503,7 @@ class ContentForSeeAlsoTest : BaseAbstractTest() { } group { //DRI should be "test//abc/#/-1/" - link { +"kotlin.collections.Collection" } + link { +"Collection" } group { group { +"Comment to collection" } } } }