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

[draft-js-import-markdown] Fix issue with complex image sources #102

Merged
merged 4 commits into from
Oct 5, 2017

Conversation

mxstbr
Copy link
Collaborator

@mxstbr mxstbr commented Oct 4, 2017

I noticed during testing that images with complex sources that contain parenthesis break. I added a couple of tests and have pinpointed the issue to the markdown parser in impport-markdown, which fails when presented with something like ![](https:///test.com/asdf(9).png). (you can see the actual failing URL in the failing test I added)

Digging into a fix now.

@mxstbr
Copy link
Collaborator Author

mxstbr commented Oct 4, 2017

Turns out somebody already submitted a PR to marked that fixes this issue 2 years ago, but it was never merged: markedjs/marked#600

Copying this fix, one sec.

Makes `href` matching of links and images non-greedy.

This fix is copied from markedjs/marked#600, and
fixes the failing tests submitted in the previous commit.
@mxstbr
Copy link
Collaborator Author

mxstbr commented Oct 4, 2017

Fixed in 0c923d1, now cleaning up linting.

@mxstbr
Copy link
Collaborator Author

mxstbr commented Oct 4, 2017

Linter fixed, code cleaned up. This is ready to merge and ship, a new release would be appreciated!

@mxstbr mxstbr changed the title [WIP] [draft-js-import-markdown] replicate issue with complex image sources [draft-js-import-markdown] replicate issue with complex image sources Oct 4, 2017
@mxstbr mxstbr changed the title [draft-js-import-markdown] replicate issue with complex image sources [draft-js-import-markdown] Fix issue with complex image sources Oct 4, 2017
@mxstbr
Copy link
Collaborator Author

mxstbr commented Oct 4, 2017

Hah shit, eslint wants me to unquote the 0 key in this code:

{
  '0': {}
}

But then flow complains "non-string literal property keys not supported". Going to add an eslint ignore statement.

@mxstbr
Copy link
Collaborator Author

mxstbr commented Oct 4, 2017

There we go, this is ready to merge!

@sstur
Copy link
Owner

sstur commented Oct 5, 2017

eslint wants me to unquote the 0 key in this code:

Yea, that is a weird edge case.

Thanks for the contribution.

@sstur sstur merged commit 503e012 into sstur:master Oct 5, 2017
@mxstbr mxstbr deleted the more-tests branch October 5, 2017 09:55
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