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

Link parsing is more lenient than either GitHub or commonmark. #456

Closed
samouri opened this issue Dec 6, 2019 · 8 comments
Closed

Link parsing is more lenient than either GitHub or commonmark. #456

samouri opened this issue Dec 6, 2019 · 8 comments
Labels
💪 phase/solved Post is done 🐛 type/bug This is a problem

Comments

@samouri
Copy link
Contributor

samouri commented Dec 6, 2019

summary
remark-parse is currently pretty lenient about whitespace within links, more so than either GitHub or commonmark. Here is an example of text that I would not expect to become a link, yet currently does:

[display text] (https://example.com)

In remark-parse, the (https://example.com) text would be considered the address of the link, even though there is a space between the two parts.

Is this intended? It feels like a bug. In particular this has been frustrating to me in its integrations with prettier, where I often am writing markdown intended for GitHub and the whitespace between the two parts is automatically stripped.

If y'all consider this a bug, I'd be happy to submit a PR to fix it!

@samouri samouri added 🐛 type/bug This is a problem 🙉 open/needs-info This needs some more info labels Dec 6, 2019
@ChristianMurphy
Copy link
Member

@samouri are you seeing this even when the option commonmark is set to true?

@samouri
Copy link
Contributor Author

samouri commented Dec 6, 2019

commonmark doesn't seem to have an effect on this. The pedantic options does net the behavior I'd expect (separating the text and link)

@wooorm
Copy link
Member

wooorm commented Dec 8, 2019

Reason for that is that GH used to support the space, but “pedantic” didn’t.

If you’re willing to work on the problem, we would accept PRs with a fix.
It’s also something that would go in #439!

@samouri
Copy link
Contributor Author

samouri commented Dec 9, 2019

What is the usual protocol for informing downstream folks of this change?

@wooorm
Copy link
Member

wooorm commented Dec 9, 2019

@samouri How do you mean? What are you worrying/wondering about?

@samouri
Copy link
Contributor Author

samouri commented Dec 9, 2019

I know that prettier is dependent on this package. This will mean a different behavior when applying prettier to markdown files with this edgecase.

I think it's a good change, but I figure any change at all could be disruptive for dependents (e.g. if they have some unit tests that start failing).

@wooorm
Copy link
Member

wooorm commented Dec 10, 2019

This is arguable* a breaking change, yes. Whether a release is breaking is signalled through semver.

* arguable because, if someone before wrote [a] (b), and prettier formatted it as [a](b), and we now stop supporting the earlier. Everything’s pretty fine 🤷‍♂️

But indeed, remark-parse is used by millions of users, and we don’t want to break their flow. We’re generally on the safe side with releases.

@samouri
Copy link
Contributor Author

samouri commented Dec 10, 2019

Thank you for the explanation! Makes sense 👍

@ChristianMurphy ChristianMurphy added 🐛 type/bug This is a problem 🙆 yes/confirmed This is confirmed and ready to be worked on and removed 🐛 type/bug This is a problem 🙉 open/needs-info This needs some more info labels Jan 7, 2020
@wooorm wooorm closed this as completed in 4f2a1d4 Mar 27, 2020
@wooorm wooorm added ⛵️ status/released and removed 🙆 yes/confirmed This is confirmed and ready to be worked on labels Mar 30, 2020
@wooorm wooorm added the 💪 phase/solved Post is done label Aug 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
💪 phase/solved Post is done 🐛 type/bug This is a problem
Development

No branches or pull requests

3 participants