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

Fix duplicate links in markdown import #161

Merged
merged 3 commits into from
Jan 29, 2019
Merged

Fix duplicate links in markdown import #161

merged 3 commits into from
Jan 29, 2019

Conversation

mxstbr
Copy link
Collaborator

@mxstbr mxstbr commented Jan 29, 2019

draft-js-import-markdown does not correctly handle the case where two links to the same src are in a line. Any ideas why @sstur?

Reference issue: https://github.com/withspectrum/spectrum/issues/4592

`draft-js-import-markdown` does not correctly handle the case where two links to the same src are in a line.
@mxstbr
Copy link
Collaborator Author

mxstbr commented Jan 29, 2019

It seems like the issue is with this.rules.link in the markdown parser. It gets greedy and returns google.com) [link2](google.com) as the href.

Digging into how to fix that 😅

This updates the _href regex to the new version from the marked codebase
to fix the bug with two links in a line: https://github.com/markedjs/marked/blob/c77a2244ab5d4a0e1dec0979a51b80df87ec6409/lib/marked.js#L574
@mxstbr
Copy link
Collaborator Author

mxstbr commented Jan 29, 2019

@sstur the latest commit fixes the bug by updating to the newest version of the _href regex from the marked codebase!!!! 🎉

https://github.com/markedjs/marked/blob/c77a2244ab5d4a0e1dec0979a51b80df87ec6409/lib/marked.js#L574

It might be time to remove the fork (if you can figure out why it was there in the first place 😅) and update marked to the latest version?

@mxstbr mxstbr changed the title Add failing test for duplicate link Fix duplicate links in markdown import Jan 29, 2019
@sstur
Copy link
Owner

sstur commented Jan 29, 2019

Awesome fix!

"It might be time to remove the fork"

Yes, agreed. It should be not a huge effort to do this.

@sstur sstur merged commit 716b505 into sstur:master Jan 29, 2019
@sstur
Copy link
Owner

sstur commented Jan 29, 2019

This has been released in draft-js-import-markdown@1.3.0.

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.

2 participants