feat(link): improve autolink behaviour with adjacent text #1812
Conversation
🦋 Changeset detectedLatest commit: 896f603 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
github-actions
bot
added
test: unit ✅
|
Deploy preview for remirror ready! ✅ Preview Built with commit 896f603. |
|
🚀 Deployed on https://1812-merge--remirror.netlify.app |
There was a problem hiding this comment.
I can see the benefits of this implementation. The issue I have with this one-pass regex approach is that it will be harder to configure.
Looking at nested braces, for example, maybe it is not a big deal for most, but it is frustrating if you need it.
I do have some ideas on making this implementation easier to configure in the future, maybe this will be even easier and will not add complexity to the extension.
I will check it out more later, I do think I see a regression. Do you mind if I make a pr into this branch with tests if I find regressions/bugs?
| @@ -51,7 +52,7 @@ const UPDATE_LINK = 'updateLink'; | |||
|
|
|||
| // Based on https://gist.github.com/dperini/729294 | |||
| const DEFAULT_AUTO_LINK_REGEX = | |||
| /(?:(?:(?:https?|ftp):)?\/\/)?(?:\S+(?::\S*)?@)?(?:(?:[\da-z\u00A1-\uFFFF][\w\u00A1-\uFFFF-]{0,62})?[\da-z\u00A1-\uFFFF]\.)*(?:(?:\d(?!\.)|[a-z\u00A1-\uFFFF])(?:[\da-z\u00A1-\uFFFF][\w\u00A1-\uFFFF-]{0,62})?[\da-z\u00A1-\uFFFF]\.)+[a-z\u00A1-\uFFFF]{2,}(?::\d{2,5})?(?:[#/?]\S*)?/gi; | |||
| /(?:(?:(?:https?|ftp):)?\/\/)?(?:\S+(?::\S*)?@)?(?:(?:[\da-z\u00A1-\uFFFF][\w\u00A1-\uFFFF-]{0,62})?[\da-z\u00A1-\uFFFF]\.)*(?:(?:\d(?!\.)|[a-z\u00A1-\uFFFF])(?:[\da-z\u00A1-\uFFFF][\w\u00A1-\uFFFF-]{0,62})?[\da-z\u00A1-\uFFFF]\.)+[a-z\u00A1-\uFFFF]{2,}(?::\d{2,5})?(?:[#/?](?:(?! |[!"'(),.;?[\]{}-]).|-+|\((?:(?![ )]).)*\)|\[(?:(?![ \]]).)*]|'(?=\w)|\.(?! |\.|$)|,(?! |,|$)|;(?! |;|$)|!(?! |!|$)|\?(?! |\?|$))+|\/)?/gi; | |||
There was a problem hiding this comment.
This looks great, I think it doesn't support some edge cases. But maybe that can be addressed later.
|
|
||
| // Expanded the link text to include additional non whitespace characters | ||
| // This determines if a link became invalid window.co -> window.confirm | ||
| const expanded = getSelectedGroup(state, /\s/, newMark) ?? newMark; |
|
Here is a similar regression that you pointed out on one of my earlier attempts. 2.mov1.mov |
|
Thanks @marcoSven, By the way, the first video doesn't seem to work? |
|
The video was just very short, updated the video. The issue is essentially the same as in the second video. I addressed this regression in #1817 |
Description
cc @marcoSven please could you provide thoughts on this implementation.
Prevent trailing punctuation being included in the the autolink URL.
Remove a link, if a URL becomes invalid i.e.
window.co->window.confirmChecklist
pnpm fixcompleted successfully.pnpm test.