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

dokkaHtml preserves teletype formatting, but dokkaGfm unhelpfully strips it #2477

Closed
cosinekitty opened this issue Apr 28, 2022 · 7 comments · Fixed by #2485
Closed

dokkaHtml preserves teletype formatting, but dokkaGfm unhelpfully strips it #2477

cosinekitty opened this issue Apr 28, 2022 · 7 comments · Fixed by #2485
Labels
bug format: gfm An issue/PR related to Dokka's GFM output format good first issue A beginner-friendly issue for which some assistance is expected

Comments

@cosinekitty
Copy link
Contributor

Describe the bug
In my Kotlin comments, I often refer to a parameter name or other code symbol using backquotes, as is typical in Markdown to render teletype/monospace text. For example, my remarks about eclipticSeparation in:

/**
 * Determines visibility of a celestial body relative to the Sun, as seen from the Earth.
 *
 * This function returns an [ElongationInfo] object, which provides the following
 * information about the given celestial body at the given time:
 *
 * - `visibility` is an enumerated type that specifies whether the body is more easily seen
 *    in the morning before sunrise, or in the evening after sunset.
 *
 * - `elongation` is the angle in degrees between two vectors: one from the center of the Earth to the
 *    center of the Sun, the other from the center of the Earth to the center of the specified body.
 *    This angle indicates how far away the body is from the glare of the Sun.
 *    The elongation angle is always in the range [0, 180].
 *
 * - `eclipticSeparation` is the absolute value of the difference between the body's ecliptic longitude
 *   and the Sun's ecliptic longitude, both as seen from the center of the Earth. This angle measures
 *   around the plane of the Earth's orbit, and ignores how far above or below that plane the body is.
 *   The ecliptic separation is measured in degrees and is always in the range [0, 180].
 *
 * @param body
 * The celestial body whose visibility is to be calculated.
 *
 * @param time
 * The date and time of the observation.
 */
fun elongation(body: Body, time: Time): ElongationInfo {
    val relativeLongitude = pairLongitude(body, Body.Sun, time)
    val elongation = angleFromSun(body, time)
    return if (relativeLongitude > 180.0)
        ElongationInfo(time, Visibility.Morning, elongation, 360.0 - relativeLongitude)
    else
        ElongationInfo(time, Visibility.Evening, elongation, relativeLongitude)
}

dokkaHtml respects this notation and converts this into

<code class="lang-kotlin">eclipticSeparation</code> is the absolute value of ...

This is the desired behavior. However, dokkaGfm strips out the backquotes and converts into plain text:

- 
   eclipticSeparation is the absolute value of ...

Expected behaviour
Backquotes should be left intact in dokkaGfm's markdown output, so that it is clear which parts of the text are prose, and which are source code excerpts. For example:

-  `eclipticSeparation` is the absolute value of ...

Screenshots
N/A

To Reproduce

  1. Clone the Astronomy Engine repository and change into its directory.
  2. git checkout -b backquote 53f5540f09df94baae112c56b9fb5066175d8a0f
  3. cd source/kotlin
  4. ./gradlew dokkaGfm dokkaHtml
  5. Edit the file source/kotlin/build/dokka/gfm/astronomy/io.github.cosinekitty.astronomy/elongation.md. Notice that the backquotes have been removed from the documented symbols "visibility", "elongation", and "eclipticSeparation".
  6. However, dokkaHtml's output preserves the intended formatting. See the corresponding file source/kotlin/build/dokka/html/astronomy/io.github.cosinekitty.astronomy/-astronomy/elongation.html. This suggests the bug is specific to dokkaGfm.

Dokka configuration
(See repro steps above. The configuration is present in the directory source/kotlin.)

Installation

  • Operating system: Happens in all 3: macOS/Windows/Linux.
  • Build tool: Gradle v7.4.1
  • Dokka version: 1.6.10

Additional context

$ ./gradlew --version

------------------------------------------------------------
Gradle 7.4.1
------------------------------------------------------------

Build time:   2022-03-09 15:04:47 UTC
Revision:     36dc52588e09b4b72f2010bc07599e0ee0434e2e

Kotlin:       1.5.31
Groovy:       3.0.9
Ant:          Apache Ant(TM) version 1.10.11 compiled on July 10 2021
JVM:          11.0.13 (JetBrains s.r.o. 11.0.13+7-b1751.25)
OS:           Linux 4.19.0-20-amd64 amd64

Are you willing to provide a PR?
I am happy to provide whatever help the maintainers could use to reproduce/diagnose this bug. Thank you!

@IgnatBeresnev IgnatBeresnev added format: gfm An issue/PR related to Dokka's GFM output format good first issue A beginner-friendly issue for which some assistance is expected labels Apr 28, 2022
@IgnatBeresnev
Copy link
Member

Low hanging fruit. Functions that render code blocks need to be overriden in CommonmarkRenderer:

override fun StringBuilder.buildCodeBlock(code: ContentCodeBlock, pageContext: ContentPage) {
    TODO("Not yet implemented")
}

override fun StringBuilder.buildCodeInline(code: ContentCodeInline, pageContext: ContentPage) {
    TODO("Not yet implemented")
}

@cosinekitty
Copy link
Contributor Author

@IgnatBeresnev, if I provide a pull request to fix this issue, is that something you would consider accepting? (Assuming of course that you approve the fix.)

@IgnatBeresnev
Copy link
Member

@cosinekitty yeah, sure, I'd be happy to review it!

I've had a look and there's only one difficult part that I missed -- you'll need it for tests. Add the following code to PageContentBuilder, somewhere close to link and comment methods:

fun codeBlock(
    language: String = "",
    kind: Kind = ContentKind.Main,
    sourceSets: Set<DokkaSourceSet> = mainSourcesetData,
    styles: Set<Style> = mainStyles,
    extra: PropertyContainer<ContentNode> = mainExtra,
    block: DocumentableContentBuilder.() -> Unit
) {
    contents += ContentCodeBlock(
        contentFor(mainDRI, sourceSets, kind, styles, extra, block).children,
        language,
        DCI(mainDRI, kind),
        sourceSets.toDisplaySourceSets(),
        styles,
        extra
    )
}

fun codeInline(
    language: String,
    kind: Kind = ContentKind.Main,
    sourceSets: Set<DokkaSourceSet> = mainSourcesetData,
    styles: Set<Style> = mainStyles,
    extra: PropertyContainer<ContentNode> = mainExtra,
    block: DocumentableContentBuilder.() -> Unit
) {
    contents += ContentCodeInline(
        contentFor(mainDRI, sourceSets, kind, styles, extra, block).children,
        language,
        DCI(mainDRI, kind),
        sourceSets.toDisplaySourceSets(),
        styles,
        extra
    )
}

You will have to run ./gradlew apiDump after this or your build will fail.

For test examples, see GroupWrapping class. For code blocks, it should look something like this

class CodeWrappingTest : GfmRenderingOnlyTestBase() {
    @Test
    fun testCodeWrapping() {
        val page = testPage {
            codeBlock {
                text("fun myCode(): String")
            }
        }
        val expect = """|//[testPage](test-page.md)
                        |
                        |```
                        |fun myCode(): String
                        |```""".trimMargin()

        CommonmarkRenderer(context).render(page)
        assertEquals(expect, renderedContent)
    }
}

I'll leave the rest (spec, corner cases, tests) to you, that's by far the most difficult part :)

For help, see CONTRIBUTING.md (it's rather poor, but better than nothing), and feel free to reach out to me in Kotlin's community slack (you can find me by my messages in #dokka channel) or via email ignat.beresnev@jetbrains.com

@cosinekitty
Copy link
Contributor Author

Thank you for all the info! I will give it a shot over the next week or so.

@cosinekitty
Copy link
Contributor Author

I'm making some progress, but I have a basic question about the classes ContentCodeBlock and ContentCodeInline. It looks like both of them are containers that hold a list of ContentNode. However, I think of these as both only having a single child in practice: the embedded literal text. I understand a code block can also contain an explicit programming language option. It seems like it should be very simple to wrap a block with 3 backquotes, the language option, newline, the raw source code, newline, then another 3 backquotes. And inline should be even simpler: wrap between backquotes. Is it safe to expect/assert exactly one child, and that child must be literal text?

@IgnatBeresnev
Copy link
Member

Nice hearing from you, I'm glad you haven't given up :)

It looks like both of them are containers that hold a list of ContentNode. However, I think of these as both only having a single child in practice: the embedded literal text.

I think you are correct in the majority of cases as it will indeed be a single text node. However, because Dokka is pluggable and the content model is universal (meaning it's the same both for commonmark and html renderers), I can imagine a scenario in which content inside ContentCodeBlock or ContentCodeInline is not just a single text node.

For instance, there might be multiple nodes that wrap text with some code highlighting or other styles, or there could even be a feature/plugin that generates clickable links for functions/classes used inside code blocks (interactive examples, if you will), or something like that. Not aware of any such examples atm, but could definitely happen.

Is it safe to expect/assert exactly one child, and that child must be literal text?

So to be on the safe side, I would just build all children. If there's only one text node, it'll should be built just fine. The rest is up to other rendering functions to implement other content types :)

(pseudo code)
text("```")
newLine()
code.children.forEach { it.build(this, pageContext) }
newLine()
text("```")

@cosinekitty
Copy link
Contributor Author

That helped a lot, thank you! I think I am close.

IgnatBeresnev pushed a commit that referenced this issue May 4, 2022
Fixes #2477.

Inline code, text that is nested within a pair of backquotes,
is now converted into GitHub Flavored Markdown (gfm) without
stripping out the backquotes. For example:

    The parameter `sum` must be a non-negative real number.

Code blocks, which are any number of lines of literal text
between triple-backquotes, and an optional programming language
name, are now preserved. If absent, the programming language
is assumed to be "kotlin". This follows the behavior of the
html renderer. For example:

    Here is an example of calling the function:
    ```kotlin
    val sum = addThemUp(left, right)
    ```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug format: gfm An issue/PR related to Dokka's GFM output format good first issue A beginner-friendly issue for which some assistance is expected
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants