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

GitHub flavored markdown: support single-tilde strikethrough #455

Merged
merged 1 commit into from
Jun 14, 2024

Conversation

camdencheek
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor Author

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
Copy link
Contributor

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
Copy link
Owner

yuin commented Jun 13, 2024

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
Copy link
Contributor

@bowman2001 Please comment.

@jmooring
Copy link
Contributor

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
Copy link

bowman2001 commented Jun 13, 2024

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
Copy link
Owner

yuin commented Jun 14, 2024

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

@yuin yuin merged commit fde4948 into yuin:master Jun 14, 2024
yuin added a commit that referenced this pull request Jun 14, 2024
yuin added a commit that referenced this pull request Jun 14, 2024
yuin added a commit that referenced this pull request Jun 14, 2024
yuin added a commit that referenced this pull request Jun 14, 2024
yuin added a commit that referenced this pull request Jun 14, 2024
yuin added a commit that referenced this pull request Jun 14, 2024
yuin added a commit that referenced this pull request Jun 14, 2024
@yuin
Copy link
Owner

yuin commented Jun 14, 2024

I've released v1.7.2.

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

@camdencheek camdencheek deleted the support-single-tilde-strikethrough branch June 14, 2024 18:25
camdencheek added a commit to sourcegraph/sourcegraph that referenced this pull request Jun 15, 2024
Updates the Goldmark markdown renderer to v1.7.2, which includes
yuin/goldmark#455, fixing the issue with
single-tilde strikethroughs not rendering as strikethroughs as described
by the Github Flavored Markdown spec.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants