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

Diff lines when deciding what blocks to update #335

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions markdown/core/api/core.api
Original file line number Diff line number Diff line change
Expand Up @@ -325,8 +325,8 @@ public abstract interface class org/jetbrains/jewel/markdown/extensions/Markdown
public final class org/jetbrains/jewel/markdown/processing/MarkdownProcessor {
public static final field $stable I
public fun <init> ()V
public fun <init> (Ljava/util/List;)V
public synthetic fun <init> (Ljava/util/List;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
public fun <init> (Ljava/util/List;Z)V
public synthetic fun <init> (Ljava/util/List;ZILkotlin/jvm/internal/DefaultConstructorMarker;)V
public fun <init> ([Lorg/jetbrains/jewel/markdown/extensions/MarkdownProcessorExtension;)V
public final fun processChildren (Lorg/commonmark/node/Node;)Ljava/util/List;
public final fun processMarkdownDocument (Ljava/lang/String;)Ljava/util/List;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package org.jetbrains.jewel.markdown.processing

import org.commonmark.node.Block
import org.commonmark.node.BlockQuote
import org.commonmark.node.BulletList
import org.commonmark.node.Code
Expand All @@ -23,6 +24,7 @@ import org.commonmark.node.SoftLineBreak
import org.commonmark.node.StrongEmphasis
import org.commonmark.node.Text
import org.commonmark.node.ThematicBreak
import org.commonmark.parser.IncludeSourceSpans
import org.commonmark.parser.Parser
import org.commonmark.renderer.text.TextContentRenderer
import org.intellij.lang.annotations.Language
Expand All @@ -41,19 +43,35 @@ import org.jetbrains.jewel.markdown.MimeType
import org.jetbrains.jewel.markdown.extensions.MarkdownProcessorExtension
import org.jetbrains.jewel.markdown.rendering.DefaultInlineMarkdownRenderer

/**
* @param optimizeEdits Optional. Indicates whether the processing should only update the changed blocks
* by keeping a previous state in memory. Default is `true`, use `false` for immutable data.
*/
@ExperimentalJewelApi
public class MarkdownProcessor(private val extensions: List<MarkdownProcessorExtension> = emptyList()) {
public class MarkdownProcessor(
private val extensions: List<MarkdownProcessorExtension> = emptyList(),
private val optimizeEdits: Boolean = true,
) {

public constructor(vararg extensions: MarkdownProcessorExtension) : this(extensions.toList())

private val commonMarkParser =
Parser.builder().extensions(extensions.map { it.parserExtension }).build()
private val commonMarkParser = Parser.builder()
.extensions(extensions.map { it.parserExtension })
.also {
obask marked this conversation as resolved.
Show resolved Hide resolved
if (optimizeEdits) {
it.includeSourceSpans(IncludeSourceSpans.BLOCKS)
}
}.build()

private val textContentRenderer =
TextContentRenderer.builder()
.extensions(extensions.map { it.textRendererExtension })
.build()

private data class State(val lines: List<String>, val blocks: List<Block>, val indexes: List<Pair<Int, Int>>)

private var currentState = State(emptyList(), emptyList(), emptyList())

/**
* Parses a Markdown document, translating from CommonMark 0.31.2
* to a list of [MarkdownBlock]. Inline Markdown in leaf nodes
Expand Down Expand Up @@ -83,11 +101,87 @@ public class MarkdownProcessor(private val extensions: List<MarkdownProcessorExt
* @see DefaultInlineMarkdownRenderer
*/
public fun processMarkdownDocument(@Language("Markdown") rawMarkdown: String): List<MarkdownBlock> {
if (!optimizeEdits) {
return textToBlocks(rawMarkdown).mapNotNull { child ->
child.tryProcessMarkdownBlock()
}
}
val (previousLines, previousBlocks, previousIndexes) = currentState
val newLines = rawMarkdown.lines()
val nLinesDelta = newLines.size - previousLines.size
// find a block prior to the first one changed in case some elements merge during the update
var firstBlock = 0
var firstLine = 0
var currFirstBlock = 0
var currFirstLine = 0
outerLoop@ for ((i, spans) in previousIndexes.withIndex()) {
obask marked this conversation as resolved.
Show resolved Hide resolved
val (_, end) = spans
for (j in currFirstLine..end) {
if (newLines[j] != previousLines[j]) {
break@outerLoop
}
}
firstBlock = currFirstBlock
firstLine = currFirstLine
currFirstBlock = i + 1
currFirstLine = end + 1
}
// find a block following the last one changed in case some elements merge during the update
var lastBlock = previousBlocks.size
var lastLine = previousLines.size
var currLastBlock = lastBlock
var currLastLine = lastLine
outerLoop@ for ((i, spans) in previousIndexes.withIndex().reversed()) {
val (begin, _) = spans
for (j in begin until currLastLine) {
if (previousLines[j] != newLines[j + nLinesDelta]) {
break@outerLoop
}
}
lastBlock = currLastBlock
lastLine = currLastLine
currLastBlock = i
currLastLine = begin
}
val updatedText = newLines.subList(firstLine, lastLine + nLinesDelta).joinToString("\n", postfix = "\n")
val updatedBlocks: List<Block> = textToBlocks(updatedText)
val updatedIndexes =
updatedBlocks.map { node ->
// special case for a bug where LinkReferenceDefinition is a Node,
// but it takes over sourceSpans from the following Block
if (node.sourceSpans.isEmpty()) {
node.sourceSpans = node.previous.sourceSpans
}
(node.sourceSpans.first().lineIndex + firstLine) to
(node.sourceSpans.last().lineIndex + firstLine)
}
val suffixIndexes = previousIndexes.subList(lastBlock, previousBlocks.size).map {
(it.first + nLinesDelta) to (it.second + nLinesDelta)
}
val newBlocks = (
previousBlocks.subList(0, firstBlock) +
updatedBlocks +
previousBlocks.subList(lastBlock, previousBlocks.size)
)
val result = newBlocks.mapNotNull { child ->
child.tryProcessMarkdownBlock()
}
val newIndexes = previousIndexes.subList(0, firstBlock) + updatedIndexes + suffixIndexes
currentState = State(newLines, newBlocks, newIndexes)
return result
}

private fun textToBlocks(strings: String): List<Block> {
val document =
commonMarkParser.parse(rawMarkdown) as? Document
commonMarkParser.parse(strings) as? Document
?: error("This doesn't look like a Markdown document")

return processChildren(document)
val updatedBlocks: List<Block> =
buildList {
document.forEachChild { child ->
(child as? Block)?.let { add(it) }
}
}
return updatedBlocks
}

private fun Node.tryProcessMarkdownBlock(): MarkdownBlock? =
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
package org.jetbrains.jewel.markdown.processing

import org.jetbrains.jewel.markdown.BlockWithInlineMarkdown
import org.jetbrains.jewel.markdown.MarkdownBlock
import org.junit.Assert.assertArrayEquals
import org.junit.Assert.assertEquals
import org.junit.Test

class MarkdownProcessorTest {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For such a big feature I would expect a ton of tests, can you please add more covering all possile cases?

E.g.:

  • First block edited
  • Last block edited
  • Block in the middle edited
  • Blocks merged
  • Blocks split
  • Blocks deleted
  • Blocks added

Etc

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PTAL #336


@Test
fun `test my processor`() {
val pp = MarkdownProcessor()
pp.processMarkdownDocument(
"""
|Paragraph 1
|
|Paragraph 2
|
|Second paragraph
|not very important
|
|* m1
|* m2
""".trimMargin(),
)
val secondProcess = pp.processMarkdownDocument(
"""
|Paragraph 1
|
|Paragraph 2
|
|Not a paragraph
|not very important
|
|* m1
|* m2
""".trimMargin(),
)
// TODO: update after changing the underlying model, to check the first elements are the same
assertEquals("Paragraph 1", (secondProcess[0] as BlockWithInlineMarkdown).inlineContent.content)
assertEquals("Paragraph 2", (secondProcess[1] as BlockWithInlineMarkdown).inlineContent.content)
assertEquals(
"Not a paragraph not very important",
(secondProcess[2] as BlockWithInlineMarkdown).inlineContent.content,
)
assertArrayEquals(
arrayOf(
"Paragraph(inlineContent=InlineMarkdown(content=m1))",
"Paragraph(inlineContent=InlineMarkdown(content=m2))",
),
(secondProcess[3] as MarkdownBlock.ListBlock).items.flatMap { it.content.map(MarkdownBlock::toString) }.toTypedArray(),
)
}
}