Correctly skip plus-prefixed URLs

[pmpinto]

Should fix #7

How: Adding a negative lookbehind on the main RegEx to ignore URLs preceded by Why: Negative lookbehinds seem to be supported since Node v6 with the --harmony flag (ref: https://node.green/#ES2018-features--RegExp-Lookbehind-Assertions) and should also be supported in ES2019 (ref: https://tc39.github.io/ecma262/).

Please note that it doesn't specifically ignore URLs that start with git+, just the + is being taken into account.

So I would suggest 1 of 2 changes: If it makes sense to apply this same rule for any other URLs that might follow the same schema but not exactly git-specific I would suggest (a) tweak the skips Git URLs test name itself. Otherwise, (b) the RegEx should be tweaked to explicitly ignore git+ URLs.

Let me know what you think.

[sindresorhus]

(a) tweak the skips Git URLs test name itself.

👍

[pmpinto]

@sindresorhus Travis CI seems to be failing in Node v6, perhaps because of the lack of the --harmony flag.

Either way, would you prefer a more compatible solution? I'm thinking something like explicitly matching the prepended + and then checking if the URL starts with a + inside linkify().

[ruipneves]

Hello 😄 You have conflicts in your branch, you should resolve them by rebasing your branch with master. Also, this package targets browsers and most of the browsers do not support lookbehinds yet. I believe that, at the moment, only the latest versions of chrome support it.

[sindresorhus]

I'm ok with targeting Node.js 10 to be able to use lookbehinds.

[pmpinto]

@ruipneves You're right, only Chrome seems to support this. @sindresorhus For this reason I pushed a new commit addressing this concern that should have the same result.

---

Sidenote: The whole commit history looks weird to me, especially with remaining conflicts. Please bear in mind that this is my first time using git rebase as well as contributing to a forked repo.

Here's what I did, IIRC:

• Committed the changes
• git remote add upstream git@github.com:sindresorhus/linkify-urls.git
• git pull upstream master
• git checkout master
• git rebase skip-prefixed-urls
• Fixed the conflicts
• Pushed, and ended up with the result you're seeing

So, how exactly should my workflow have been for this?

[satazor]

The lookbehind was more elegant but the current PR is indeed more widely supported in terms of browsers.

@sindresorhus can you give your opinion regarding the current state of the PR vs the lookbehind strategy? Bear in mind that there's no babel plugin able to transform lookbehind for older browsers, see: http://kangax.github.io/compat-table/es2016plus/#test-RegExp_Lookbehind_Assertions

[satazor]

@pmpinto regarding the conflicts, try the following:

git pull upstream master --rebase
<resolve conflicts>
git commit
git push origin skip-prefixed-urls

[sindresorhus]

I still prefer the lookbehind approach. This is a niche module and I would prefer to keep it as simple as possible. I'm not that concerned with browser support. We can add a note to the readme to stay on v2 if people needs to support different browsers.

[pmpinto]

@satazor Thanks for the Git tips! I'm still not confident by looking at the whole commit history, but the current state is the intended one. Still not sure what went wrong... What's your usual rebase flow?

@sindresorhus Thanks for your input, just updated the RegEx, tests and README file. Good to go from my side 👍

[satazor]

@pmpinto It would have been a good practice to rebase all the commits to just one by using git rebase -i, but @sindresorhus will probably hit the Squash button instead of merging. This will basically create a single commit on the master branch. This a GitHub feature though and might not be available in other services.

[vasco-santos]

@pmpinto considering that @sindresorhus is ok with not supporting node@6, please do not forget to change the travis configuration appropriately

[pmpinto]

@vasco-santos Thanks for the heads up, Vasco!

[sindresorhus]

It would have been a good practice to rebase all the commits to just one by using git rebase -i,

I actually prefer to keep the history intact to more easily see what changed. I squash on merge regardless.

[sindresorhus]

Looks good! Nice work, @pmpinto :)