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

Missing space between link and bold #2446

Closed
Arthur-Milchior opened this issue Apr 18, 2022 · 5 comments
Closed

Missing space between link and bold #2446

Arthur-Milchior opened this issue Apr 18, 2022 · 5 comments
Assignees
Labels

Comments

@Arthur-Milchior
Copy link

Describe the bug

As an example, look at the source https://github.com/Kotlin/kotlinx.coroutines/blob/master/kotlinx-coroutines-core/common/src/flow/operators/Share.kt#L390 and more specifically the line:

Returns a flow that invokes the given [action] after this shared flow starts to be collected

Screenshots

On https://kotlin.github.io/kotlinx.coroutines/kotlinx-coroutines-core/kotlinx.coroutines.flow/-shared-flow/index.html you'll get the sentence

Returns a flow that invokes the given actionafter this shared flow starts to be collected (after the subscription is registered).

2022-04-18-123230_601x142_scrot

Expected behaviour

There should be a space between "action" and "after". (I checked, this line was last modified two years ago, so it's not just that some recent change did not propagate to the documentation website)

Dokka configuration

No idea. It's the one used by the official kotlin documentation website

Are you willing to provide a PR?

Probably not. No idea where to start looking

@IgnatBeresnev IgnatBeresnev self-assigned this Apr 18, 2022
@IgnatBeresnev
Copy link
Member

IgnatBeresnev commented Apr 18, 2022

Thank you for reporting it, surprised we haven't noticed it earlier and don't have test to check this particular case :)

I've been able to reproduce it with a simple example

/**
 * A _simple_ [List] example
 */
class Example {}

2022-04-18_14-02-39

It seems like the space gets lost if there's a link after some text formatting (bold/italic/etc).

@Arthur-Milchior
Copy link
Author

You are welcome. Glad it was easy to reproduce, hopefully it'll make debugging easier.
Good luck with that

@IgnatBeresnev
Copy link
Member

IgnatBeresnev commented Apr 18, 2022

I've had some time to debug it, thinking it was an easy fix somewhere in the HTML renderer. Unfortunatley, this looks a bit scary to fix now for 1.6.21 as it might introduce more bugs, so we'll get back to it later.

I'll post some technical details down below to help fixing this in the future.

Take the following kdoc as an example:

/**
 * A _simple_ [List] example
 */

It breaks down into

[0] = LeafAstNode TEXT, "A"
[1] = LeafAstNode WHITE_SPACE, " "
[2] = CompositeAstNode EMPH, "_simple_" -- later "simple" is turned into text and _ symbols are substituted for style
[3] = LeafAstNode WHITE_SPACE, " "
[4] = CompositeAstNode SHORT_REFERENCE_LINK, with 3 children: [, List, ]

What happens is MarkdownParser#mergedLeafNode tries to merge consecutive leaf nodes into a single leaf node, which is supposed to turn

[0] = LeafAstNode TEXT, "A"
[1] = LeafAstNode WHITE_SPACE, " "

into a single

[0] = LeafAstNode TEXT, "A " (notice the whitespace at the end)

It does so only for consecutive leaf nodes, so [Composite, Leaf, Leaf, Composite] turns into [Composite, Leaf, Composite]. However, if all the leaf nodes between the composite ones are evaluated to empty text, there's an if that just ignores it:

if (text.substring(startOffset, endOffset).transform().trim().isNotEmpty()) {
    ... merging logic ...
}
return null

so it results in [Composite, Composite]. This is the first place to fix. I've removed this if to see what happens -- tests pass, but the problem doesn't go away.

Even if we leave this single white space leaf node be (that is not remove it), there's another place that removes elements with single whitespaces down the road: DocTagsFromIElementFactory#getInstance, which calls parseWithNormalisedSpaces, which in turn calls parseHtmlEncodedWithNormalisedSpaces. That's the second place where whitespaces get lost.

I haven't dug into it further, but there may be more problems related to this.

@IgnatBeresnev IgnatBeresnev added this to the Stable milestone Apr 19, 2022
@atyrin
Copy link
Contributor

atyrin commented Apr 20, 2022

Also relevant issue: #1975

@IgnatBeresnev
Copy link
Member

Let's close this as a duplicate in favour of #1975 - it has more info on reproducing it.

Once again thanks for the report!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants