-
Notifications
You must be signed in to change notification settings - Fork 179
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
Added xmpp and mailto support to the autoprefixer extension #274
Conversation
Should the loop break instead of continue on a protocol? It looks like this would accept several: |
Most constructs are parsed “normally”, such as the protocols here, instead of postprocessing the text like emails here. |
No, the protocols only work when surrounded by non-alphanumeric characters so the above example would result in the following: <p>mailto:aaaxmpp:<a href="mailto:aaa@stuff.com">aaa@stuff.com</a></p>
Unfortunately that's not possible here due to the way the autolinker finds email addresses. We could catch the protocols there and apply them, yes, but it would then also apply the automatic mailto on top of it when it matches on the two emails that now exist. We need to handle the email-like protocols within the email section of the autolinker specifically to prevent this from happening. |
I don’t understand your second point. Postprocessing text, where email detection happens, excludes links. It doesn’t link, say, |
@wooorm Ah so it does. Good catch, I'll look into implementing those changes. Thanks! |
@wooorm I spent some time over the past two days exploring moving the protocol validation out of the I was already at double the code length just replicating the validation, and we'd have to write the email matching again from scratch (or at least pull it out into it's own function). Where it currently sits the email matching is already happening, so the simplest way to get this working is as I've currently made the change. I do agree it would be nice to do it that way you're suggesting, but the added development time isn't worth it when the current process handles the new protocol additions perfectly already. |
Huh, weird that it is so complex! I thought you’d have to do very little validation. Because, when prefixed with
...So I’d imagine that it would only be adding |
It sounds like you are trying to implement the more strict proper parsing that happens for emails, where’s I’m thinking more: because there’s such an unlikely-to-occur-in-prose protocol already, it can go straight to Alas, I’d hope that this loose |
The problem there is that an email isn't just a domain, so it's not so simple to just use that function. The domain part works for everything after the Either way it's more a question of "where" the code should sit, and not really functionality. As this currently stands the code works in the many varied test cases I've provided, and so I don't know that there is much more to be gained by refactoring this out to use I do appreciate the feedback through and it did give me a chance to explore other options which is always a positive. |
Will this be added to the autolink spec? |
I second that it is very important to update the spec for these things. |
cmark-gfm
's autlinker extension detects email addresses and automatically converts them intomailto:
links. The xmpp protocol also contains addresses that look semantically identical to email addresses, and so are being mislinked as emails.Example markdown and output:
Instead the desired output would be as so:
This allows both xmpp and mailto protocols to be specified directly, and the autolinker should be smart enough to handle both of those, and ignore any other protocols so we don't break any existing functionality (such as someone typing
Send an email to:me@domain.tld
, for example).The changes have been made and tests updated to reflect the change in spec.