search

kramdown / parser-gfm

kramdown-parser-gfm provides a kramdown parser for the GFM dialect of Markdown
Other
57 stars 14 forks source link

Strikethrough not conforming with CommonMark rules #14

Open brodock opened 5 years ago

brodock commented 5 years ago

Here are two examples with their expected output:

Input: 

This ~~is a complex *strike* through *test ~~with nesting~~ involved* here~~.

Output:

<p>This <del>is a complex <em>strike</em> through <em>test <del>with nesting</del> involved</em> here</del>.</p>

Input:

This ~~is a complex *strike* through *test~~ without nesting ~~involved* here~~.

Output:

<p>This <del>is a complex <em>strike</em> through *test</del> without nesting <del>involved* here</del>.</p>

Current implementation:

STRIKETHROUGH_DELIM = /~~/.freeze
STRIKETHROUGH_MATCH = /#{STRIKETHROUGH_DELIM}(?!\s|~).*?[^\s~]#{STRIKETHROUGH_DELIM}/m.freeze

Is able to successfully perform the first, but not the second case.

Here is an example running the two on commonmark:

I believe I'm on the right direction here, but so far what I've manage to do was get the second example to work by breaking the first one.

What I did was base on the balanced parameters example, using some lookahead and lookbehind code:

STRIKETHROUGH_MATCH = /(?<re>~~(?!\s|~)((?:.*)|\g<re>)?(?<!\s|~)~~)/m.freeze
ashmaroli commented 5 years ago

This case has been explicitly documented by the author as expected behavior But, based on observations above, the results are quite different.

Should we update the gfm-parser to mimic GitHub's current output or not is up for discussion. /cc @gettalong

gettalong commented 5 years ago

The original GFM parser targeted the GFM implementation of Github, not the CommonMark re-implementation. So you will have to make a trade-off between backwards compatibility and targeting the current CommonMark based implementation.

I'm fine if the parser gets changed to follow the current GFM implementation. Note, however, that this may involve quite some changes.

ashmaroli commented 5 years ago

This is not reproducible on master. If you're facing this issue with kramdown-parser-gfm-1.0.1, rest assured as the next release will have it fixed.

Closing..

ashmaroli commented 5 years ago

Nope. This report is still valid..