Add `All Types` page generation #3267

whyoleg · 2023-10-27T10:36:43Z

Fixes #2887

Notes:

All Types page (and link to it) is not generated when there are no types (classes, type aliases, etc) to document in module
Link to page is located under all packages in navigation page (HTML format only) and under the Packages block on module page for all formats
If documentation or tag (f.e. SinceKotlin) on type differs depending on sourceSet - we don't show documentation or tag for this type
Path to all page relative to root of the module:
- module: **/htmlMultiModule/kotlinx-coroutines-core/index.html
- all types: **/htmlMultiModule/kotlinx-coroutines-core/all-types.html

Preview of how it will look like with kotlinx-coroutines codebase:

All Types page

Link to All Types page on module page

When there are no types in module

Since Kotlin

IgnatBeresnev · 2023-10-30T14:17:30Z

Superb work, thank you for the description and the screenshots!

If documentation on type (or SinceKotlin tag) differs depending on sourceSet - we don't show it

Huh, we indeed forgot about this corner case. I wonder if "no documentation" is better than "some documentation", even if "some documentation" is not complete.

What I mean is: right now, if common and jvm documentation differs, we don't show anything. But what if we show common's documentation in this case? I would expect it to still be correct and provide at least some basic information, without implementation details.

@ilya-g do you have any thoughts on that? Maybe some stdlib cases come to mind where it would be beneficial or undesirable?

Path to all page relative to root of the module

Is there a way to avoid the whitespace in the filename, and maybe have a different filename altogether? That is, have the page be "All Types", but the html file be all-types.html (without the upper-case prefixes)? Or is it super tightly coupled?

IgnatBeresnev · 2023-10-30T14:40:29Z

Link to page is located under all packages in navigation page (HTML format only)

This is also a bit of a technical limitation, right?

If I remember correctly, the navigation tree resets (or even throws an error) if you visit an html page that has no line in the navigation sidemenu. If this is the case, then removing the All Types link from the sidemenu might require additional and somewhat unrelated technical debt work. We also forgot about it while discussing this issue

whyoleg · 2023-10-30T14:44:28Z

Regarding documentation block selection:
Now it uses similar (a little improved) approach which was used for packages description, though different documentation for packages are much harder to achieve comparing to type documentations.
What I spotted in kx.coroutines is:

CancellationException has no documentation in common, but has a little different documentation in shared source sets - so no documentation is shown
Runnable has the same documentation for all declarations, but because it actualised by typealias on JVM something was lost somewhere

Is there a way to avoid the whitespace in the filename

Found how to fix this in rather simple way, will do, thanks for pointing on it!

whyoleg · 2023-10-30T14:57:47Z

This is also a bit of a technical limitation, right?

Yes and no I believe. Because all-types page is child of module page, we add it to children of module page, and as so it appears on the same level with packages and automatically added to navigation page. I haven't tried to remove it during navigation page generation (may be it will be not so hard), as I thought that may be it's fine to show it there, as IMO it's better for UX to somehow show on which page user is located now in nav bar

ilya-g · 2023-10-31T16:00:17Z

What I mean is: right now, if common and jvm documentation differs, we don't show anything. But what if we show common's documentation in this case? I would expect it to still be correct and provide at least some basic information, without implementation details.
do you have any thoughts on that? Maybe some stdlib cases come to mind where it would be beneficial or undesirable?

In the old dokka it was hard coded to something like "if there's no common documentation, use the one from JVM". I think too that in this case it would be better to show some summary (e.g. common documentation).

whyoleg · 2023-11-01T15:46:06Z

Update:

hide All Types page under a flag dokka.shouldDisplayAllTypesPage (can be enabled in the same way as sinceKotlin (could be revisited in scope of Introduce extra options for plugin configuration #2776)
documentation for type now shown in any case (if it's exists), with priority to common documentation
also I've reverted some accidental formatting changes from previous commits, so it should be easier to review PR

plugins/base/src/main/kotlin/translators/documentables/DefaultPageCreator.kt

vmishenev · 2023-10-27T19:16:24Z

plugins/base/src/main/kotlin/translators/documentables/DefaultPageCreator.kt

+                typelike.descriptions[sourceSet]?.let { sourceSet to it }
+            }.distinctBy { it.second }.singleOrNull()
+
+            val customTags = typelike.customTags.values.mapNotNull { sourceSetTag ->


Does it make sense to show all custom tags, e.g. author, throws and so on?

As far as I see from code, author and throws are not custom tags.
In our codebase there are 2 custom tags provider: SinceKotlin and Mathjax
Still, may be we really need to show SinceKotlin only

As far as I see from code, author and throws are not custom tags.

Oh, sorry, I forgot(

Still, may be we really need to show SinceKotlin only

I think it should be consistent with a list of types in a regular page (package, classlike ..) so showing all custom tags is ok.

Looks as if it can get SinceKotlin and the description from different source sets, which might be misleading, as there might be cases when a type's actual was added later (like ConcurrentModificationException)

I think the logic here needs to be predetermined and intentional. I can think of two solutions, but both have drawbacks

Get SinceKotlin from the same source set as the description. Drawback: if the description is taken from JVM, common's SinceKotlin might be lower/higher.

Get the earliest SinceKotlin. Drawback: it might be inconsistent with the text in the description.

Wouldn't hurt consulting Ilya about this

Still, may be we really need to show SinceKotlin only

I don't think it's a problem for now, and hardcoding it is less than ideal, so I'd leave it as is

@ilya-g what are your thoughts regarding comment above about best (more correct) way of showing SinceKotlin/description when they are different per source set?

I'd propose to do the same as in the old dokka: showing the minimum version among those remained after filtering with platform selector.

Thanks! Now we take minimum SinceKotlin version in All Types page. Implemented in d744e49

Though, it's not affected by platform selector and will only show minimum version based on all platforms, as it looks like it's not possible to show different content based on different set of platform selectors.

Also I've checked how current stdlib documentation behaves, and looks like it doesn't show SinceKotlin at all if they are different per sourceSet + no changes after filtering by platform.
Here is an example for kotlin.ConcurrentModificationException:

With all platforms selected

With only Native selected

So I would propose to go for now without additional filtering based on selected platforms. And implement it later if needed. @ilya-g WDYT?

We discussed this question with Oleg, and it looks like we indeed don't have the technical ability to change the value dynamically based on platform selectors.

Implementing it would take considerable time, and it doesn't seem to be that critical for the initial release, so let's just merge the PR with the currently implemented and consistent logic: always show the earliest version

We can change it later on if it becomes apparent that it's misleading or is a problem

plugins/base/src/main/kotlin/translators/documentables/DefaultPageCreator.kt

IgnatBeresnev

We discussed most of these in voice, so I didn't go into too much detail. Leaving the comments to not forget about something

plugins/base/src/main/kotlin/translators/documentables/DefaultPageCreator.kt

plugins/base/src/main/kotlin/renderers/html/NavigationDataProvider.kt

core/src/main/kotlin/pages/PageNodes.kt

plugins/base/src/main/kotlin/renderers/html/HtmlRenderer.kt

core/src/main/kotlin/model/Documentable.kt

plugins/base/src/main/kotlin/translators/documentables/DefaultPageCreator.kt

plugins/base/src/test/kotlin/content/annotations/SinceKotlinTest.kt

IgnatBeresnev · 2023-11-07T13:34:34Z

plugins/base/src/main/kotlin/translators/documentables/DefaultPageCreator.kt

+                typelike.descriptions[sourceSet]?.let { sourceSet to it }
+            }.distinctBy { it.second }.singleOrNull()
+
+            val customTags = typelike.customTags.values.mapNotNull { sourceSetTag ->


Looks as if it can get SinceKotlin and the description from different source sets, which might be misleading, as there might be cases when a type's actual was added later (like ConcurrentModificationException)

I think the logic here needs to be predetermined and intentional. I can think of two solutions, but both have drawbacks

Get SinceKotlin from the same source set as the description. Drawback: if the description is taken from JVM, common's SinceKotlin might be lower/higher.

Get the earliest SinceKotlin. Drawback: it might be inconsistent with the text in the description.

Wouldn't hurt consulting Ilya about this

Still, may be we really need to show SinceKotlin only

I don't think it's a problem for now, and hardcoding it is less than ideal, so I'd leave it as is

plugins/base/src/test/kotlin/content/AllTypesPageTest.kt

IgnatBeresnev

Good job and thank you for addressing so many review comments!

...a-subprojects/plugin-base/src/main/kotlin/org/jetbrains/dokka/base/pages/AllTypesPageNode.kt

Co-authored-by: Ignat Beresnev <ignat.beresnev@jetbrains.com>

IgnatBeresnev · 2023-11-23T11:26:33Z

...ain/kotlin/org/jetbrains/dokka/base/transformers/pages/annotations/SinceKotlinTransformer.kt

+            Platform.wasm to SinceKotlinVersion("1.8"),
+        )
+
+        fun minSinceKotlinVersionOfPlatform(platform: Platform): SinceKotlinVersion {


nit: since this was moved into SinceKotlinVersion, I think it can be renamed to be just minVersionOfPlatform, so that the call looks like SinceKotlinVersion.minVersionOfPlatform() instead of duplicating SinceKotlinVersion.minSinceKotlinVersionOfPlatform()

IgnatBeresnev · 2023-11-23T11:35:05Z

...ain/kotlin/org/jetbrains/dokka/base/transformers/pages/annotations/SinceKotlinTransformer.kt

+            // the structure of custom tag content for SinceKotlin should be in sync
+            // with how DefaultPageCreator.contentForAllTypes reads it


Also not that critical, but if the two parts must be in sync, it makes sense to have them in a single place. For instance,

// inside one class or file and next to each other, maybe inside SinceKotlinVersion fun createSinceKotlinCustomTag(): CustomTagWrapper { ... } fun extractSinceKotlinFromCustomTag(customTagWrapper: CustomTagWrapper): SinceKotlinVersion? {}

The benefit of having these two things next to each other is that you see clearly the correlation between the two, and if one changes - it'll be easier to find and remember to change the other. And you'll be able to reference it in the KDoc

* Hide All Types page under a system property * Show documentation if it exists selecting most relevant * Take the minimum SinceKotlin version if they differ * Extract internal configuration properties to one place

whyoleg requested review from ilya-g, vmishenev and IgnatBeresnev October 27, 2023 10:36

whyoleg self-assigned this Oct 27, 2023

whyoleg force-pushed the all-types branch from 76e66ac to 0598517 Compare November 1, 2023 15:43

vmishenev reviewed Nov 1, 2023

View reviewed changes

IgnatBeresnev reviewed Nov 7, 2023

View reviewed changes

whyoleg requested review from vmishenev and IgnatBeresnev November 13, 2023 13:06

vmishenev approved these changes Nov 14, 2023

View reviewed changes

IgnatBeresnev approved these changes Nov 15, 2023

View reviewed changes

...a-subprojects/plugin-base/src/main/kotlin/org/jetbrains/dokka/base/pages/AllTypesPageNode.kt Show resolved Hide resolved

...a-subprojects/plugin-base/src/main/kotlin/org/jetbrains/dokka/base/pages/AllTypesPageNode.kt Show resolved Hide resolved

whyoleg added 13 commits November 20, 2023 14:35

Implement All Types page

45bea03

Fix tests after All Types page addition

70ccbc4

Add some content tests for All Types and module pages

7c3805e

Update api dump

0a9db9d

Fix name of generated all-types page file

ee1eda1

Hide All Types page under a system property

aacae67

Revert some formatting changes

2b28d46

Add link to issue about feature toggles

ee0e5f9

Add test for all pages sorting

444444b

Show documentation if it exists selecting most relevant

444aaba

add one more test

b8080b2

Improve code around AllTypesPageNode construction after PR review

00ef9d9

Revert public API changes

7376b2d

whyoleg and others added 10 commits November 20, 2023 14:35

Revert logic changes for packages

a6cbd1e

cleanup DefaultPageCreator based on PR review

a984c03

extract internal configuration properties to one place

b946360

explicit flow in sorting of navigation page

ad2be97

rename buildRowForModule function

7f8a50c

remove println from plugin-base tests

bd6458e

Add documentation for AllTypesPageNode

6c5c25e

Co-authored-by: Ignat Beresnev <ignat.beresnev@jetbrains.com>

Add a note for copy pasted declaration

661fc57

Co-authored-by: Ignat Beresnev <ignat.beresnev@jetbrains.com>

Split AllTypesPage tests

bd114f0

Take the minimum SinceKotlin version if they differ for AllTypesPage

d744e49

whyoleg force-pushed the all-types branch from e776ce6 to d744e49 Compare November 20, 2023 12:44

IgnatBeresnev approved these changes Nov 23, 2023

View reviewed changes

Put SinceKotlinVersion related functions near each other

ec328d2

whyoleg merged commit 17ad866 into master Nov 24, 2023
9 checks passed

whyoleg deleted the all-types branch November 24, 2023 09:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `All Types` page generation #3267

Add `All Types` page generation #3267

whyoleg commented Oct 27, 2023 •

edited

Loading

IgnatBeresnev commented Oct 30, 2023

IgnatBeresnev commented Oct 30, 2023

whyoleg commented Oct 30, 2023

whyoleg commented Oct 30, 2023

ilya-g commented Oct 31, 2023

whyoleg commented Nov 1, 2023 •

edited

Loading

vmishenev Oct 27, 2023

whyoleg Nov 2, 2023

vmishenev Nov 2, 2023 •

edited

Loading

IgnatBeresnev Nov 7, 2023

whyoleg Nov 13, 2023

ilya-g Nov 16, 2023

whyoleg Nov 20, 2023

IgnatBeresnev Nov 23, 2023 •

edited

Loading

IgnatBeresnev left a comment

IgnatBeresnev Nov 7, 2023

IgnatBeresnev left a comment

IgnatBeresnev Nov 23, 2023

IgnatBeresnev Nov 23, 2023

		// the structure of custom tag content for SinceKotlin should be in sync
		// with how DefaultPageCreator.contentForAllTypes reads it

Add All Types page generation #3267

Add All Types page generation #3267

Conversation

whyoleg commented Oct 27, 2023 • edited Loading

IgnatBeresnev commented Oct 30, 2023

IgnatBeresnev commented Oct 30, 2023

whyoleg commented Oct 30, 2023

whyoleg commented Oct 30, 2023

ilya-g commented Oct 31, 2023

whyoleg commented Nov 1, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vmishenev Nov 2, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

IgnatBeresnev Nov 23, 2023 • edited Loading

Choose a reason for hiding this comment

IgnatBeresnev left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

IgnatBeresnev left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Add `All Types` page generation #3267

Add `All Types` page generation #3267

whyoleg commented Oct 27, 2023 •

edited

Loading

whyoleg commented Nov 1, 2023 •

edited

Loading

vmishenev Nov 2, 2023 •

edited

Loading

IgnatBeresnev Nov 23, 2023 •

edited

Loading