Updated webLinks regex accept colons, fix vscode issue#60401

[jrkong]

@Tyriar This PR should fix issue #60401 for VS Code downstream.

I updated the webLinks pathClause regex

xterm.js/src/addons/webLinks/webLinks.ts

Line 17 in b9ab35d

const pathClause = '(\\/[\\/\\w\\.\\-%~]*)*';

to accept colons. So the new regex is now (\\/[\\/\\w\\.\\-%~:]*)*

I also updated the tests to test for the colon case in the path.

If there are any updates I should make please let me know.

[Tyriar]

Would it be possible to allow semi-colons except for the last character?

So this row:

const row = '  http://foo.com/a~b#c~d?e~:f:  ';

Resulted in this:

assert.equal(uri, 'http://foo.com/a~b#c~d?e~:f');

[jrkong]

@Tyriar I've been testing it out and I don't think it's doable, at least not through regex. I can't think of a way to implement an exception case into the capture group so that it accepts all : except for the last character.

I was thinking of handling the case by processing the uri after it's processed by strictUrlRegex. Would that be a viable solution? If so how would I access the uri before it's returned? I couldn't to access it properly/safely during my tests.

If you have any other ideas on how to implement this please let me know.

[Tyriar]

The concern is that then we will break on this type of link microsoft/vscode#50426 (comment) (I know it's a file not a URL but this just popped up in my inbox). It's hard to say which is more common 😕

[jrkong]

Is that the only case we need to be concerned with as I don't think it will be an issue.

In VS Code the path regex is managed by these lines in terminalLinkHandler.ts in VS Code.

This results in the regex

/(((\.\.?|\~)|([^\0\s!$`&*()\[\]+'":;\\])+)?(\/([^\0\s!$`&*()\[\]+'":;\\])+)+)(((\S*)",[  ]line[  ]((\d+)([  ]column[  ](\d+))?))|((\S*)[  ]on[  ]line[  ]((\d+)(,[  ]column[  ](\d+))?))|((\S*):line[  ]((\d+)(,[  ]column[  ](\d+))?))|(([^\s\(\)]*)(\s?[\(\[](\d+)(,\s?(\d+))?)[\)\]])|(([^:\s\(\)<>'"\[\]]*)(:(\d+))?(:(\d+))?))/

in Javascript.

In comparison to

/(?:^|[^\da-z\.-]+)((https?:\/\/)((([\da-z\.-]+)\.([a-z\.]{2,6}))|((\d{1,3}\.){3}\d{1,3})|(localhost))(:\d{1,5})?(\/[\/\w\.\-%~]*)*(\?[0-9\w\[\]\(\)\/\?\!#@$%&'*+,:;~\=\.\-]*)?(#[0-9\w\[\]\(\)\/\?\!#@$%&'*+,:;~\=\.\-]*)?)($|[^\/\w\.\-%]+)/

which is used when a web link is detected.

Are there any other cases that this change could break?

[Tyriar]

I mean this form could appear in URLs as well though, for example:

https://go.microsoft.com/fwlink/?LinkId=blah: some description

Will a [^:\s]* similar to the path regex not work for webLinks?

[jrkong]

@Tyriar fixed! [^:\s] worked. I completely overlooked the \s character as I was doing my tests! Thanks for pointing that out! I've added tests as well to verify the functionality of both cases.

[jrkong]

@Tyriar can you check if the PR is missing any edge cases or tests? Colons at the end of URLs are ignored now but I'm not sure if I'm missing anything else that needs to be accounted for.

[Tyriar]

Sorry about the delay. This was a little unexpected but other than that it looks like a great compromise:

It seems to take all colons in the string if there are colons not at the end.