JetBrains / markdown

Markdown parser written in kotlin
Apache License 2.0
682 stars 75 forks source link

Fix incorrect parsing of intersecting `*` and `~` #100

Closed FirstTimeInForever closed 2 years ago

FirstTimeInForever commented 2 years ago

The old implementation was incorrectly parsing sequences of intersecting * and ~ resulting in broken tree (#98 and IDEA-280541). The problem was that both EmphStrongParser and StrikeThroughParser were not aware of each other, and it was impossible to determine whether there is an unbalanced delimiter that should break a current element. The new implementation runs in three passes (should be still linear, since it is based on cmark and markdown-it implementations and using the same search optimizations):

This fixes parsing of sequences with intersecting */~ and some corner cases that were previously parsed incorrectly. Changed test were checked with CommonMark dingus and GitHub markdown editor.

I suggest not to open new .md test files in IDEA, since it currently breaks indices and the whole IDE.

valich commented 2 years ago

Thank you very much! Can I ask you to review and update Spec tests (CommonMarkSpecTest.kt, GfmSpecTest.kt) by removing @ignored from the ones which are fixed now after your changes? Also, have you made any measurements on performance changes?

FirstTimeInForever commented 2 years ago

Also, have you made any measurements on performance changes?

Included performanceTest gradle task reports quite similar results with/without my changes (fastest old/slowest new based on few runs):

gitBook: 10.241056ms/10.452499ms
commonMarkSpec: 19.176091ms/19.218096ms
gitBook: 2.946399ms/2.958774ms
fogChangelog: 13.156232ms/13.433774ms
commonMarkSpec: 41.615804ms/41.167164ms

Although, I think new implemntation should be generally slower (not drastically) due to additional steps and allocations.

As for @ignored tests, I need to check them - I had no idea about them.

Not sure why macos build is broken, but it seems to be some issue with kotlin or configuration.

valich commented 2 years ago

Thanks! #96 fixes macos tests, they are green now. I'll merge your PR now.

FirstTimeInForever commented 2 years ago

@valich Is it possible to release new version with those changes so we can use it in IDEA?

valich commented 2 years ago

Yes, I'll do this soon.

FirstTimeInForever commented 2 years ago

Thank you!

valich commented 2 years ago

Available in 0.3.0