Relative paths and paths without columns not linked in debug console

[felixfbecker]

• VSCode Version: 1.5, 1.6 insider
• OS Version: Windows 10

Paths from the stack trace are not clickable

[felixfbecker]

Btw this also affects relative paths:

I would consider this more of a bug than a feature request, as it works correctly on the other OS. This is really slowing down productivity, please please consider this for the next iteration. I imagine it is just a wrong regexp somewhere.

[isidorn]

Here is a code pointer
PRs that fix this are welcome. If it turns out to be too complex I can look into fixing this in the future

[felixfbecker]

Thanks @isidorn I will take a look at this! Are there tests for this?

[isidorn]

@felixfbecker no but adding tests would be even more super awesome.
It should be possible to create a ReplExpressionsRenderer with a MockEditorService and test out the link detection. If this seems unintersting I can look into adding tests in one of our debt weeks.

P.S this is good documentation on how to run vscode out of source and immediatly try out your changes https://github.com/Microsoft/vscode/wiki/How-to-Contribute
This change should not break the existing OS X and linux behavior - not sure if you have those machines but I can check it on my VMs easily

[isidorn]

Also a bit related #6301

And make sure to get the very latest - now I polished the imports in that file so you might end up in merge conflicts if you start on a pr on an older code base.

[felixfbecker]

Fixed the regexp so that

• relative paths are matched (the "root" path is optional now)
• paths without a column are matched ("column" path is optional now, as PHP only reports line numbers)
• no line breaks and colons are allowed inside the path (otherwise change 2 would cause unintended matches)

Need to build VS Code now and find out if the match groups that may now be undefined cause any problems. Also need to test URIs.

[felixfbecker]

I also noticed only group 1, 3 and 4 are used. Can I remove capturing of group 2 (root component)?

[isidorn]

@felixfbecker I believe that group is there for a reason. Though @bpasero originally added it
Since you are now being less strict about link detection try to verify if you introduced false positives.
Once your change is in a good shape just create a PR and ping me on it - thanks!

[felixfbecker]

Well I checked this with Find all references and the regexp is only used in a single place, where only the capture groups I mentioned are used. But I'm gonna leave it out for now.

[bpasero]

These regexes are literally taken from our Azure version I think and super old. Feel free to revisit them, but make sure to see the test cases and update them as well 👍

[roblourens]

This is now not working in Insiders on MacOS, maybe from this change?

Stable, links underlined and clickable:

Insiders, highlighted in the error message but not the stack trace, and that one link tries to open as a path relative to the workspace, then shows an error message because it can't find the file:

[roblourens]

Actually I see the same thing in Windows. I think it's just highlighting the first link it finds instead of all of them. On Windows the link is still clickable though.

[aeschli]

Also no success for me. Shouldn't this work? No links for me (Windows 10)

console.error('C:\\Users\\aeschli\\workspaces\\samples\\main.js:1');
console.error('C:/Users/aeschli/workspaces/samples/main.js:1');
process.exit();

[roblourens]

It seems like it's only supposed to work with paths in parens and with :#:# like you'd find in a stack trace

[felixfbecker]

Yes, or line start / line end should work too. I have no idea though why only the first link is highlighted and don't have time currently to investigate

[roblourens]

Investigated, and there are sort of two problems:

• Clicking on an underlined link doesn't work in MacOS
  • https://github.com/Microsoft/vscode/blob/master/src/vs/workbench/parts/debug/electron-browser/replViewer.ts#L384
  • The link detection regex isn't capturing the leading '/', so that line sends it to toResource which concatenates it to the workspace root. I'm not sure what's wrong with the regex.
• Only the first link in a log message is shown
  • I can't figure out where this came in, but I know it was introduced somewhere between
  • 62ed382 (merging Resolve relative REPL paths #15464)
  • and d2a4e2e (binary searching is getting too time-consuming)
  • But somewhere, we started parsing an entire message at a time for a link, instead of just one line at a time

[isidorn]

Thanks @roblourens for a great investigation. The first issue is introduced by the last PR. The second issue is introduced by a refactoring on my end where we try to handle new lines in the debug console better.

I have pushed a safest fix that takes care of those two problems - reverted only the last PR and disabled the grouping together of the output. This should now bring us to the same behavior of the last stable release (with potentialy some improvements by the first two PRs).

Our link handling has many flaws and I plan to tackle this in the January release.

[isidorn]

After testing this out it seems we have regressed a bit. Namely when there is one large output event we only detect the first link. This is practice means when a stack trace is printed that only the top stack trace element can be clicked.
If there are mutliple stack traces printed to the console then the first link in each stack trace will be detected.

I am not sure how big of a regression this is. This was working before becuase we were actually not handling the output events properly and we were separting them in new lines wrongly.

@weinand can decide if it is ok to ship with this limitation. If it is not ok to ship with this then I propose we go back to the old spliting each output per line.
In any case I plan to revisit how we do this whole link detection in January

[felixfbecker]

Why not split it at the time the regexp is applied?

[isidorn]

@felixfbecker yeah that is also an option. But when there are multiple links detected on one element than the formating / newlines get screwed up. So there would some additional changes required which are never smart to do in last minue fixes such as this one :)

[isidorn]

@felixfbecker unforuntetly had to revert all your prs because we decided to go back to the behavior we had in vscode stable.

Since we have polluted the history of this bug I have created a new one and assigned it to January so we can properly fix this in the next release. Here is the issue #17085

[isidorn]

For verification:

Verify vscode insiders is handling links in the repl the same way vscode stable is doing right now

[roblourens]

Thanks, looks good to me on MacOS in master (needs verification in a real build)