[JEWEL-600] Use chars indexes for markdown edits #2922
Conversation
|
TODO: address Jacob's suggestions: Please consider renaming variables or extracting some functionality into smaller functions that explain themselves in the title, and/or adding some more larger comments. e.g. all the stuff about prefix, suffix, and positions doesn't really make sense in the context of processing a string which represents markdown imo. Perhaps textMatchingBeforeEdits and and textMatchingAfterEdits, would be more appropriate. Additionally, firstBlock could be firstMarkdownBlockThatNeedsProcessed or firstChangedMarkdownBlock A comment explaining what the function is doing at the high level would help a lot too. e.g. "Process markdown quickly by comparing it to the previously processed markdown and discarding text that matched before the edits and after the edits by doing string comparisons on the text. After processing update our state again with our newly processed markdown blocks." (or something) or perhaps just a comment showing an example would help e.g. "abcdefg" vs "abcefg". |
rock3r
requested review from
AlexVanGogen and
rock3r
and removed request for
AlexVanGogen
|
@obask please always preface PR titles with |
obask
changed the title
obask
force-pushed
the
use-char-indexes-instead-of-lines
branch
3 times, most recently
from
23ad03d to
56452a3
Compare
...l/markdown/core/src/main/kotlin/org/jetbrains/jewel/markdown/processing/MarkdownProcessor.kt
Outdated
Show resolved
Hide resolved
...l/markdown/core/src/main/kotlin/org/jetbrains/jewel/markdown/processing/MarkdownProcessor.kt
Outdated
Show resolved
Hide resolved
|
@obask Is it still a draft? Are there any other substantial changes planned? |
|
Yesterday he said he'd rebase to solve the CI issues and mark as ready to review. @obask can you please do it? |
obask
force-pushed
the
use-char-indexes-instead-of-lines
branch
2 times, most recently
from
151ba05 to
738b568
Compare
obask
force-pushed
the
use-char-indexes-instead-of-lines
branch
from
738b568 to
4794382
Compare
|
@obask static analysis is failing, can you take a look? |
obask
force-pushed
the
use-char-indexes-instead-of-lines
branch
from
4794382 to
2b8a55f
Compare
Format project doesn't rearrange (star) imports, so I did it in an IDE. It should work now. |
|
There is still some static analysis issue (and yes, star imports are only really addressed by the IDE, because ktfmt doesn't have access to the classpath to resolve these imports) |
Our previouis version was marking a previouis and the following blocks as modified, while it's not needed in most cases. We can use use 0.24.0 inputIndex parameter to simply keep an index from char to block and avoid splitting lines and doing complicated search for a modified substring.
obask
force-pushed
the
use-char-indexes-instead-of-lines
branch
from
2b8a55f to
eee09a3
Compare
Our previouis version was marking a previouis
and the following blocks as modified, while it's
not needed in most cases.
We can use use 0.24.0 inputIndex parameter to
simply keep an index from char to block and avoid
splitting lines and doing complicated search for
a modified substring.