Fix weblinks

[jerch]

This PR tries to tackle several issues regarding weblinks:

• fix underline offsets
• allow more broad url scheme (unicode, with name and password portion)
• use URL() to validate instead of bloated regexp

TODO:

• find better fix for DOM renderer issue
• properly test underline offsets in API tests
• fix hover/underline state on wrapped urls not activating offtopic here, created Issue in Linkifier2._removeIntersectingLinks #4296
• better trimming to max 2048 chars

Closes #4294.
Closes #3539.
Closes #4247.
Closes #3721.
Closes #2887.

[jerch]

@Tyriar Do you happen to have an url test battery in vscode (some list of naughty url constellations), so we could check for more edge cases or things, that should explicitly match or not match as url?

[jerch]

FIXME: Also exclude any sort of brackets from url scheme: <>()[]{}

[jerch]

@Tyriar Imho this is ready for review.

Note that I changed the way how urls are matched - I removed the old explicit positive matching with an exclude matching to allow for newer unicode based urls (supported by all major browsers). Ofc with exclude matching the chance is higher to wrongly match too much, but thats something we can still adjust later on.

The WebLinkProvider._mapStrIdx method might be code we want to move to Buffer later on (maybe replacing the partially broken Buffer.stringIndexToBufferIndex), as we need that functionality in several places.

[jerch]

@Tyriar Thx to gh suggestions I just found out about https://github.com/microsoft/vscode-uri

Could we just use that as well in the weblinks addon? I guess that it would simplify alot and avoid lengthy discussions in follow-up issues about treating this or that as valid/invalid url?

[Tyriar]

vscode-uri is good, but I think it also has some quirks like always encoding the URL when sometimes it's not desirable (eg. encoding [ and ]). Could try that out to see how much we get from it? It's an addon so I'm not too concerned about a dependency, especially if it's from our team.

[Tyriar]

I'm guessing people will complain about regressions here, interesting learning about rfc1738 though. Should we be more explicit about the intended scope of the addon in the readme? I've always viewed it as a decent basic URL detector, but it's not meant to be feature complete and handle a bunch of edge cases.

[jerch]

I'm guessing people will complain about regressions here

Yes thats the downside of turning the matcher logic upside-down, but I did not find another way while allowing these unicode uris to match. Will see if I get less "overmatching" from the vscode-uri package.

[jerch]

Not sure how to go about the risk of overmatching. I think the new matcher is way more capable than the old one, so I'd suggest to go with new one and lets see, if ppl stumble into tons of issues?

And if thats a big issue for peeps, the old addon version would still be around and work as before, so we can fix things iteratively?

[jerch]

@Tyriar Oh well, vscode-uri does not help here much, initially I thought it brings a method to identify url offsets within a string, but it is basically an enhanced new URL() nodejs version, but for that we can just stick to the browser's impl. So nothing to be gained here from vscode-uri.

[Tyriar]

lgtm, I'll make a note on the release notes about how this is a big change