-
Notifications
You must be signed in to change notification settings - Fork 42
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Fix Markdown rendering issues (#458)
* Make Markdown processing not optimised by default The vast majority of Markdown use cases will be displaying static text, so trying to optimise for an editor-like scenario is counter-productive. This PR sets the flag to false by default. This commit also cleans up the MarkdownProcessor code and kdocs. * Make Markdown blocks implement equals and hashcode The value classes we used to use would rely on the CommonMark models for this, but none of the CommonMark models actually implement equals() and hashCode(), leading to issues in tests and in Compose. * Cleanup links in JewelReadme Now links point to GitHub, so the app doesn't crash when you click them. * Add equals/hashcode to inline Markdown entities, too Tests have been updated to match the new behaviour, that's closer to the CommonMark one. Now we don't carry unparsed inline Markdown anymore, but rather we fully parse it in advance, so rendering it later is easier and doesn't require running parsing in the UI anymore. This is a tradeoff in memory usage for speed of rendering, and it makes sense in the context of most Markdown text being static and only parsed once, then kept on screen. Inline extensions are now possible, but not tested. This makes #325 possible, since the extension now can be moved to its own module. Note: please don't look too much at the test changes. They're _verbose_, and are only there to ensure CommonMark specs compliance. * Add inline extensions mechanism This is not tested, and not used yet, so it may or may not need tweaks down the line. We'll find out when we implement the first extension.
- Loading branch information
Showing
30 changed files
with
4,327 additions
and
2,928 deletions.
There are no files selected for viewing
Large diffs are not rendered by default.
Oops, something went wrong.
148 changes: 66 additions & 82 deletions
148
markdown/core/src/main/kotlin/org/jetbrains/jewel/markdown/InlineMarkdown.kt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,96 +1,80 @@ | ||
package org.jetbrains.jewel.markdown | ||
|
||
import org.commonmark.node.Node | ||
import org.jetbrains.jewel.markdown.InlineMarkdown.Code | ||
import org.jetbrains.jewel.markdown.InlineMarkdown.CustomNode | ||
import org.jetbrains.jewel.markdown.InlineMarkdown.Emphasis | ||
import org.jetbrains.jewel.markdown.InlineMarkdown.HardLineBreak | ||
import org.jetbrains.jewel.markdown.InlineMarkdown.HtmlInline | ||
import org.jetbrains.jewel.markdown.InlineMarkdown.Image | ||
import org.jetbrains.jewel.markdown.InlineMarkdown.Link | ||
import org.jetbrains.jewel.markdown.InlineMarkdown.SoftLineBreak | ||
import org.jetbrains.jewel.markdown.InlineMarkdown.StrongEmphasis | ||
import org.jetbrains.jewel.markdown.InlineMarkdown.Text | ||
import org.commonmark.node.Code as CMCode | ||
import org.commonmark.node.CustomNode as CMCustomNode | ||
import org.commonmark.node.Emphasis as CMEmphasis | ||
import org.commonmark.node.HardLineBreak as CMHardLineBreak | ||
import org.commonmark.node.HtmlInline as CMHtmlInline | ||
import org.commonmark.node.Image as CMImage | ||
import org.commonmark.node.Link as CMLink | ||
import org.commonmark.node.SoftLineBreak as CMSoftLineBreak | ||
import org.commonmark.node.StrongEmphasis as CMStrongEmphasis | ||
import org.commonmark.node.Text as CMText | ||
import org.jetbrains.jewel.foundation.GenerateDataFunctions | ||
|
||
/** | ||
* A run of inline Markdown used as content for | ||
* [block-level elements][MarkdownBlock]. | ||
*/ | ||
public sealed interface InlineMarkdown { | ||
public val nativeNode: Node | ||
|
||
@JvmInline | ||
public value class Code(override val nativeNode: CMCode) : InlineMarkdown | ||
|
||
@JvmInline | ||
public value class CustomNode(override val nativeNode: CMCustomNode) : InlineMarkdown | ||
|
||
@JvmInline | ||
public value class Emphasis(override val nativeNode: CMEmphasis) : InlineMarkdown | ||
|
||
@JvmInline | ||
public value class HardLineBreak(override val nativeNode: CMHardLineBreak) : InlineMarkdown | ||
|
||
@JvmInline | ||
public value class HtmlInline(override val nativeNode: CMHtmlInline) : InlineMarkdown | ||
|
||
@JvmInline | ||
public value class Image(override val nativeNode: CMImage) : InlineMarkdown | ||
|
||
@JvmInline | ||
public value class Link(override val nativeNode: CMLink) : InlineMarkdown | ||
|
||
@JvmInline | ||
public value class SoftLineBreak(override val nativeNode: CMSoftLineBreak) : InlineMarkdown | ||
@GenerateDataFunctions | ||
public class Code(override val content: String) : InlineMarkdown, WithTextContent | ||
|
||
public interface CustomNode : InlineMarkdown { | ||
/** | ||
* If this custom node has a text-based representation, this function | ||
* should return it. Otherwise, it should return null. | ||
*/ | ||
public fun contentOrNull(): String? = null | ||
} | ||
|
||
@JvmInline | ||
public value class StrongEmphasis(override val nativeNode: CMStrongEmphasis) : InlineMarkdown | ||
@GenerateDataFunctions | ||
public class Emphasis( | ||
public val delimiter: String, | ||
override val inlineContent: List<InlineMarkdown>, | ||
) : InlineMarkdown, WithInlineMarkdown { | ||
public constructor( | ||
delimiter: String, | ||
vararg inlineContent: InlineMarkdown, | ||
) : this(delimiter, inlineContent.toList()) | ||
} | ||
|
||
@JvmInline | ||
public value class Text(override val nativeNode: CMText) : InlineMarkdown | ||
public data object HardLineBreak : InlineMarkdown | ||
|
||
@GenerateDataFunctions | ||
public class HtmlInline(override val content: String) : InlineMarkdown, WithTextContent | ||
|
||
@GenerateDataFunctions | ||
public class Image( | ||
public val source: String, | ||
public val alt: String, | ||
public val title: String?, | ||
override val inlineContent: List<InlineMarkdown>, | ||
) : InlineMarkdown, WithInlineMarkdown { | ||
public constructor( | ||
source: String, | ||
alt: String, | ||
title: String?, | ||
vararg inlineContent: InlineMarkdown, | ||
) : this(source, alt, title, inlineContent.toList()) | ||
} | ||
|
||
public val children: Iterable<InlineMarkdown> | ||
get() = | ||
object : Iterable<InlineMarkdown> { | ||
override fun iterator(): Iterator<InlineMarkdown> = | ||
object : Iterator<InlineMarkdown> { | ||
var current = this@InlineMarkdown.nativeNode.firstChild | ||
@GenerateDataFunctions | ||
public class Link( | ||
public val destination: String, | ||
public val title: String?, | ||
override val inlineContent: List<InlineMarkdown>, | ||
) : InlineMarkdown, WithInlineMarkdown { | ||
public constructor( | ||
destination: String, | ||
title: String?, | ||
vararg inlineContent: InlineMarkdown, | ||
) : this(destination, title, inlineContent.toList()) | ||
} | ||
|
||
override fun hasNext(): Boolean = current != null | ||
public data object SoftLineBreak : InlineMarkdown | ||
|
||
@GenerateDataFunctions | ||
public class StrongEmphasis( | ||
public val delimiter: String, | ||
override val inlineContent: List<InlineMarkdown>, | ||
) : InlineMarkdown, WithInlineMarkdown { | ||
public constructor( | ||
delimiter: String, | ||
vararg inlineContent: InlineMarkdown, | ||
) : this(delimiter, inlineContent.toList()) | ||
} | ||
|
||
override fun next(): InlineMarkdown = | ||
if (hasNext()) { | ||
current.toInlineNode().also { | ||
current = current.next | ||
} | ||
} else { | ||
throw NoSuchElementException() | ||
} | ||
} | ||
} | ||
@GenerateDataFunctions | ||
public class Text(override val content: String) : InlineMarkdown, WithTextContent | ||
} | ||
|
||
public fun Node.toInlineNode(): InlineMarkdown = | ||
when (this) { | ||
is CMText -> Text(this) | ||
is CMLink -> Link(this) | ||
is CMEmphasis -> Emphasis(this) | ||
is CMStrongEmphasis -> StrongEmphasis(this) | ||
is CMCode -> Code(this) | ||
is CMHtmlInline -> HtmlInline(this) | ||
is CMImage -> Image(this) | ||
is CMHardLineBreak -> HardLineBreak(this) | ||
is CMSoftLineBreak -> SoftLineBreak(this) | ||
is CMCustomNode -> CustomNode(this) | ||
else -> error("Unexpected block $this") | ||
} |
109 changes: 51 additions & 58 deletions
109
markdown/core/src/main/kotlin/org/jetbrains/jewel/markdown/MarkdownBlock.kt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,90 +1,83 @@ | ||
package org.jetbrains.jewel.markdown | ||
|
||
import org.commonmark.node.Block | ||
import org.commonmark.node.Heading as CMHeading | ||
import org.commonmark.node.Paragraph as CMParagraph | ||
import org.jetbrains.jewel.foundation.GenerateDataFunctions | ||
|
||
public sealed interface MarkdownBlock { | ||
public data class BlockQuote(val children: List<MarkdownBlock>) : MarkdownBlock | ||
@GenerateDataFunctions | ||
public class BlockQuote(public val children: List<MarkdownBlock>) : MarkdownBlock { | ||
public constructor(vararg children: MarkdownBlock) : this(children.toList()) | ||
} | ||
|
||
public sealed interface CodeBlock : MarkdownBlock { | ||
public val content: String | ||
|
||
public data class IndentedCodeBlock( | ||
override val content: String, | ||
) : CodeBlock | ||
@GenerateDataFunctions | ||
public class IndentedCodeBlock(override val content: String) : CodeBlock | ||
|
||
public data class FencedCodeBlock( | ||
@GenerateDataFunctions | ||
public class FencedCodeBlock( | ||
override val content: String, | ||
val mimeType: MimeType?, | ||
public val mimeType: MimeType?, | ||
) : CodeBlock | ||
} | ||
|
||
public interface CustomBlock : MarkdownBlock | ||
|
||
@JvmInline | ||
public value class Heading( | ||
private val nativeBlock: CMHeading, | ||
) : MarkdownBlock, BlockWithInlineMarkdown { | ||
override val inlineContent: Iterable<InlineMarkdown> | ||
get() = nativeBlock.inlineContent() | ||
|
||
public val level: Int | ||
get() = nativeBlock.level | ||
@GenerateDataFunctions | ||
public class Heading( | ||
override val inlineContent: List<InlineMarkdown>, | ||
public val level: Int, | ||
) : MarkdownBlock, WithInlineMarkdown { | ||
public constructor(level: Int, vararg inlineContent: InlineMarkdown) : this(inlineContent.toList(), level) | ||
} | ||
|
||
public data class HtmlBlock(val content: String) : MarkdownBlock | ||
@GenerateDataFunctions | ||
public class HtmlBlock(public val content: String) : MarkdownBlock | ||
|
||
public sealed interface ListBlock : MarkdownBlock { | ||
public val children: List<ListItem> | ||
public val isTight: Boolean | ||
|
||
public data class OrderedList( | ||
@GenerateDataFunctions | ||
public class OrderedList( | ||
override val children: List<ListItem>, | ||
override val isTight: Boolean, | ||
val startFrom: Int, | ||
val delimiter: String, | ||
) : ListBlock | ||
|
||
public data class UnorderedList( | ||
public val startFrom: Int, | ||
public val delimiter: String, | ||
) : ListBlock { | ||
public constructor( | ||
isTight: Boolean, | ||
startFrom: Int, | ||
delimiter: String, | ||
vararg children: ListItem, | ||
) : this(children.toList(), isTight, startFrom, delimiter) | ||
} | ||
|
||
@GenerateDataFunctions | ||
public class UnorderedList( | ||
override val children: List<ListItem>, | ||
override val isTight: Boolean, | ||
val marker: String, | ||
) : ListBlock | ||
public val marker: String, | ||
) : ListBlock { | ||
public constructor( | ||
isTight: Boolean, | ||
marker: String, | ||
vararg children: ListItem, | ||
) : this(children.toList(), isTight, marker) | ||
} | ||
} | ||
|
||
public data class ListItem( | ||
val children: List<MarkdownBlock>, | ||
) : MarkdownBlock | ||
|
||
public object ThematicBreak : MarkdownBlock | ||
|
||
@JvmInline | ||
public value class Paragraph(private val nativeBlock: CMParagraph) : MarkdownBlock, BlockWithInlineMarkdown { | ||
override val inlineContent: Iterable<InlineMarkdown> | ||
get() = nativeBlock.inlineContent() | ||
@GenerateDataFunctions | ||
public class ListItem(public val children: List<MarkdownBlock>) : MarkdownBlock { | ||
public constructor(vararg children: MarkdownBlock) : this(children.toList()) | ||
} | ||
} | ||
|
||
public interface BlockWithInlineMarkdown { | ||
public val inlineContent: Iterable<InlineMarkdown> | ||
} | ||
|
||
private fun Block.inlineContent(): Iterable<InlineMarkdown> = | ||
object : Iterable<InlineMarkdown> { | ||
override fun iterator(): Iterator<InlineMarkdown> = | ||
object : Iterator<InlineMarkdown> { | ||
var current = this@inlineContent.firstChild | ||
|
||
override fun hasNext(): Boolean = current != null | ||
public data object ThematicBreak : MarkdownBlock | ||
|
||
override fun next(): InlineMarkdown = | ||
if (hasNext()) { | ||
current.toInlineNode().also { | ||
current = current.next | ||
} | ||
} else { | ||
throw NoSuchElementException() | ||
} | ||
} | ||
@GenerateDataFunctions | ||
public class Paragraph( | ||
override val inlineContent: List<InlineMarkdown>, | ||
) : MarkdownBlock, WithInlineMarkdown { | ||
public constructor(vararg inlineContent: InlineMarkdown) : this(inlineContent.toList()) | ||
} | ||
} |
5 changes: 5 additions & 0 deletions
5
markdown/core/src/main/kotlin/org/jetbrains/jewel/markdown/WithInlineMarkdown.kt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
package org.jetbrains.jewel.markdown | ||
|
||
public interface WithInlineMarkdown { | ||
public val inlineContent: List<InlineMarkdown> | ||
} |
5 changes: 5 additions & 0 deletions
5
markdown/core/src/main/kotlin/org/jetbrains/jewel/markdown/WithTextContent.kt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
package org.jetbrains.jewel.markdown | ||
|
||
public interface WithTextContent { | ||
public val content: String | ||
} |
25 changes: 25 additions & 0 deletions
25
...c/main/kotlin/org/jetbrains/jewel/markdown/extensions/MarkdownInlineProcessorExtension.kt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
package org.jetbrains.jewel.markdown.extensions | ||
|
||
import org.commonmark.node.CustomNode | ||
import org.jetbrains.jewel.markdown.InlineMarkdown | ||
import org.jetbrains.jewel.markdown.processing.MarkdownProcessor | ||
|
||
public interface MarkdownInlineProcessorExtension { | ||
/** | ||
* Returns true if the [node] can be processed by this extension instance. | ||
* | ||
* @param node The [CustomNode] to parse | ||
*/ | ||
public fun canProcess(node: CustomNode): Boolean | ||
|
||
/** | ||
* Processes the [node] as a [InlineMarkdown.CustomNode], if possible. Note | ||
* that you should always check that [canProcess] returns true for the same | ||
* [node], as implementations might throw an exception for unsupported node | ||
* types. | ||
*/ | ||
public fun processInlineMarkdown( | ||
node: CustomNode, | ||
processor: MarkdownProcessor, | ||
): InlineMarkdown.CustomNode? | ||
} |
24 changes: 24 additions & 0 deletions
24
...rc/main/kotlin/org/jetbrains/jewel/markdown/extensions/MarkdownInlineRendererExtension.kt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
package org.jetbrains.jewel.markdown.extensions | ||
|
||
import org.jetbrains.jewel.markdown.InlineMarkdown | ||
import org.jetbrains.jewel.markdown.InlineMarkdown.CustomNode | ||
import org.jetbrains.jewel.markdown.rendering.InlineMarkdownRenderer | ||
|
||
/** | ||
* An extension for [InlineMarkdownRenderer] that can render one or more | ||
* [InlineMarkdown.CustomNode]s. | ||
*/ | ||
public interface MarkdownInlineRendererExtension { | ||
/** Check whether the provided [inline] can be rendered by this extension. */ | ||
public fun canRender(inline: CustomNode): Boolean | ||
|
||
/** | ||
* Render a [CustomNode] as an annotated string. Note that if | ||
* [canRender] returns `false` for [inline], the implementation might throw. | ||
*/ | ||
public fun render( | ||
inline: CustomNode, | ||
inlineRenderer: InlineMarkdownRenderer, | ||
enabled: Boolean, | ||
) | ||
} |
Oops, something went wrong.