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

[commonmark]+[gfm] make autolinks compliant #1034

Merged
merged 11 commits into from
Jan 24, 2018

Conversation

Feder1co5oave
Copy link
Contributor

@Feder1co5oave Feder1co5oave commented Jan 23, 2018

This fixes #1027 and replaces #638, fixes #842, fixes #813, fixes #859, fixes #593, fixes #890

It follows the commonmark spec and gfm extension spec.

  • commonmark autolinks must be enclosed in < > and must be absolute URIs or email addresses.
  • gfm extended autolinks are recognized without enclosing < > and can be http, https or ftp absolute URLs, URLs starting with 'www.' and email addresses.

Example 574 of the cm_autolinks test fails due to encoding problems, they'll be fixed in the next PR.

@joshbruce
Copy link
Member

Thanks @Feder1co5oave! Seems to be failing??

#8. cm_autolinks failed at offset 1232. Near: "<p><ahref="http://example.com/\[\">http://example.com/\[\</a".


Got:
<p><ahref="http://example.com/\[\">http://example.com/\[\</a


Expected:
<p><ahref="http://example.com/%5C%5B%5C">http://example.com/

@Feder1co5oave
Copy link
Contributor Author

Feder1co5oave commented Jan 23, 2018

Yep, I explained in my first post:

Example 574 of the cm_autolinks test fails due to encoding problems, they'll be fixed in the next PR.

I actually could have edited the test case to make it pass, but I think it's time to solve this urlencoding thing so I left it as it was in commonmark. Commonmark does url encoding.

@Feder1co5oave
Copy link
Contributor Author

I'm thinking Travis CI integration with this repo would be really, really convenient, since you can set it up to run at every commit on master and on every pull request submitted. That way you can run tests automatically and get immediate feedback, and you could also do linting if you will.
Is there no chance that chjj could take this up and set it on the repo? After having enabled it, we could tweak the "build" process by simply committing to the source base.

@joshbruce
Copy link
Member

Sorry, missed that. Thanks for the reiteration. Agree with the CI piece - unfortunately, I don't control that. See #967 for more details.

@joshbruce joshbruce merged commit eeb7ac5 into markedjs:master Jan 24, 2018
@Feder1co5oave Feder1co5oave deleted the new_autolink branch January 25, 2018 16:06
zhenalexfan pushed a commit to zhenalexfan/MarkdownHan that referenced this pull request Nov 8, 2021
[commonmark]+[gfm] make autolinks compliant
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants