Debug console link detection issues

[isidorn]

Also consider a way for extensions to contribute link detection rules

[DanTup]

@sandy081 I'm just trying to step through this code to understand why my links aren't being detected, but I'm really confused by this code:

https://github.com/Microsoft/vscode/blob/master/src/vs/workbench/parts/debug/browser/linkDetector.ts#L48-L53

			while (match !== null) {
				let resource: uri = null;
				if (!resource) {
					match = pattern.exec(text);
					continue;
				}

As far as I can tell, the rest of the code in this block will never execute - resource is always initialised as null then the if statement continues?

[DanTup]

In fact, it looks like maybe the minifier stripped all the code out because of that?

[DanTup]

@isidorn Is anyone able to review the comments above? Am I misreading the code or is this a bug that means this will never work? :(

[isidorn]

@DanTup yes this seems like a clear bug on our end. Would you be interested in providing a PR which fixes this.
This was initialy done by our previous internt @michelkaporin

[DanTup]

@isidorn I haven't contributed any real code before, do I need to (and if so, are there instructions on how to) run Code from source? (I checked contributing.md but it doesn't seem to cover it). Also, are there automated tests around this sort of functionality I could extend?

[isidorn]

@DanTup https://github.com/Microsoft/vscode/wiki/How-to-Contribute
We do not have tests around link detection unfortunetly, so you would have to manually test it out

@gregvanl our contributing.md should have a pointer to our wiki how to contribute page imho

[DanTup]

I did have this on my list to try and look at, but I never gotten around to it. I think it's unlikely to happen anytime soon so it may be better for someone else to look at.

(I do a lot of people would benefit in fixing - many issues have been closed as dupes of this, though is in the backlog marked as debt, when the above is really a bug).

[DanTup]

@isidorn Coming back to this, I noticed fixing the detection won't work for us, as our links have been rewritten as URIs, like:

flutter: #0      _GalleryAppState.build (package:flutter_gallery/gallery/app.dart:131:5)
flutter: #1      StatefulElement.build (package:flutter/src/widgets/framework.dart:3809:27)

Was there any progress on allowing extensions to contribute rules for link detection? (I couldn't find anything)

[isidorn]

No porgress here

[mymikemiller]

I just made a Pull Request that brings this feature back (#65929). I'm curious to hear your thoughts, @isidorn.

[DanTup]

FWIW, while setting up a repro using the mock debugger today, I noticed that OutputEvents can be given source locations (it didn't show in the completion until I cast it like new OutputEvent(${message}\n, category) as OutputEvent & DebugProtocol.OutputEvent.

So, I realised I could regex my paths out of the call stacks and just attach them to each line this way.

Before (no links):

After (links):

As an advantage, this allows mapping the package: URIs to the correct source file, and also allows truncating the visible file path if desired to take up less space.

[jribbens]

I notice that the link detection has changed in release 1.75.0 - and from my point of view the changes have made it worse. I frequently have links that look like https://example.com/path(abc,def), and previously this link would be correctly detected in the terminal (but not in the debug console, where the final ) would be omitted)... but now in the terminal the link is wrongly terminated before the ,.

[roblourens]

Terminal link detection is not related to this, please file a separate issue @jribbens

[jribbens]

@roblourens What is this ticket about then? If it's about the debug console link detection, then my comment from April 2021 still stands - given a URL like https://example.com/path(abc,def) the debug console omits the close bracket from the link.

[roblourens]

It's about the debug console, but if I understand your comment correctly, you are saying that something in the terminal regressed?

[jribbens]

The debug console behaves the same as it has for a long time, in that for URLs of the form I mentioned (which I use a lot), it fails to include the final ). Obviously "URL guessing" can never be perfect, but I expected VSCode's debug tab to get it right, because VSCode's terminal tab got it right. The recent regression is that the terminal tab now gets it even wronger than the debug tab, and terminates the URL at the ,. So previously one tab got it right and one got it wrong, now they both get it wrong, but in different ways ;-)

[roblourens]

Yes, sharing link detection implementation would also be a good thing for us to get to. It would be helpful to file an issue for the terminal regression so that we can fix that.

[jribbens]

@roblourens Ok I've opened a separate issue #174009

[ngduchuan]

FWIW, while setting up a repro using the mock debugger today, I noticed that OutputEvents can be given source locations (it didn't show in the completion until I cast it like new OutputEvent(${message}\n, category) as OutputEvent & DebugProtocol.OutputEvent.

So, I realised I could regex my paths out of the call stacks and just attach them to each line this way.

Before (no links):

After (links):

As an advantage, this allows mapping the package: URIs to the correct source file, and also allows truncating the visible file path if desired to take up less space.

@DanTup I also face this problem, but I didn't understand how you did. Could you please guide it in some detail? Many thanks

[DanTup]

@ngduchuan are you an extension author, or a Flutter user? If an extension author, you need to set the source field on your OutputEvents. If you're a Flutter user and not seeing these links, please ensure you're on the latest version of Flutter and if you're not seeing links you'd expect, please file an issue at https://github.com/Dart-Code/Dart-Code.

[MarByteBeep]

I still have issues with link detection (and specifically Ctrl Clicking the link) in vscode as well. For instance vscode does seem to detect the following link, and I'm able to Ctrl click it

TypeError: fetch failed
    at fetch (C:\git\myproject\node_modules\undici\index.js:115:13)

But when prefixed with file:///, the link is recognized, but I'm not able to Ctrl click it:

    at async apiCall (file:///C:/git/myproject/src/lib/api.js:284:19)

Any thoughts?

[DanTup]

But when prefixed with file:///, the link is recognized, but I'm not able to Ctrl click it:

 at async apiCall (file:///C:/git/myproject/src/lib/api.js:284:19)

This is probably related to #150702. VS Code doesn't handle file:/// URIs that have line/col on the end using colons. The argument is that the colons could be part of the filename. Personally I think it's the wrong call given how many languages are outputting links in this format.

[MarByteBeep]

Yes, especially since the first one (on the call stack) has a "correct" url, but ends up deep in the nodejs library. The actual link that would make sense for me to focus on would be the second one. And that one cannot be clicked. Not ideal 😉

[DanTup]

Have there been changes around this? In the latest release notes I see:

File protocol URIs (file://) now support line and column number extensions at the end, like most other links. The new # format is also supported.

But I can't find any issue or PR related to this. I'm specifically interested in what "line and column number extensions" mean. Is file:///foo:1:2 finally supported, or is this meant to be a description of #193384 (which is not about file URIs, but file paths ending with ranges)?

[roblourens]

In the release notes, that is specifically for the terminal. No progress in the debug console, unfortunately.

[DanTup]

@roblourens do you know where the Issue/PR for the change that was made is? I'm just about the specifics of it because I'd opened some issues like #150702 (which I think might be a terminal now?) with issues in this area, but it's not clear if this fix helps (the suggestion was that colons on a file:/// URI are not allowed because they're valid as part of a filename, and I'm wondering if this stance has changed).

[roblourens]

#193629