yuin / goldmark

:trophy: A markdown parser written in Go. Easy to extend, standard(CommonMark) compliant, well structured.
MIT License
3.68k stars 255 forks source link

GitHub flavored markdown: support single-tilde strikethrough #455

Closed camdencheek closed 4 months ago

camdencheek commented 4 months ago

According to the GitHub Flavored Markdown spec, strikethrough can use either one or two tildes. Currently, the Strikethrough extension only parses the two tilde form. This makes a tiny modification to reduce the minimum delimiter size for the Strikethrough extension so that it can correctly parse the single-tilde form.

jmooring commented 4 months ago

I am not in favor of this change; I think GitHub got it wrong.

Some applications support subscripts via single tilde encapsulation (e.g., H~2~O). We already have strikethrough support, and the existing syntax has been in place for years. In my view the risk of syntax collision outweighs the benefit of this change.

If this proposal is accepted, please make single tilde support optional and disabled by default.

camdencheek commented 4 months ago

Whether or not GitHub got it wrong (and I don't necessarily disagree), the feature is advertised as conforming to GitHub Flavored Markdown, and is included in the extensions.GFM set. Given that, a mismatch with the spec seems to me like it should be pretty straightforward to categorize this as a bug in the extension. I expect most people want the GFM extensions to match the behavior of GitHub as closely as possible.

That said, for my purposes, making it an off-by-default configurable option on the Strikethrough extension would be fine so I'm happy to defer here 🤷

jmooring commented 4 months ago

I expect most people want the GFM extensions to match the behavior of GitHub as closely as possible.

First, the existing behavior has been in place for years, and this is the first request that I am aware of that wants that to change.

Second, as one of the largest (if not the largest) downstream consumers of goldmark, some Hugo users will be harmed by this change unless it's configurable and is disabled by default.

yuin commented 4 months ago

I probably merge this PR as a default behavior. 'Defined by official spec' is strong support for this.

If Hugo want to stay ~a~ unchanged, they can make your extensions higher priority. StrikeThrough extension has a priority 500, so they can set like priority 400. Any problems? @jmooring

jmooring commented 4 months ago

@bowman2001 Please comment.

jmooring commented 4 months ago

I'll let bowman2001 comment on prioritization, which to me seems fine. But regardless of whether a downstream project has a conflicting extension, this is a breaking change.

bowman2001 commented 4 months ago

Thanks for the notification, @jmooring.

When we would set the priority of the subscript (~a~) parser higher than the one for the strike-through (~~a~~) parser, the subscript parser will consume both tildes from a strike-through as a double subscript.

This change would definitely prohibit to use the strike-through and the subscript extension together, as it is currently possible. Therefore, I would also suggest not implementing this detail of the GFM spec or making it only optional.

yuin commented 4 months ago

Strikethrough is just an extension. Need to control details, you can replace this extension with your extension.

yuin commented 4 months ago

I've released v1.7.2.

Maybe a subscript extension with a higher priority can cooperate with the strikethrough extension.