Honestly I'm not sure this is the best approach. The whole link opening code seems to be a total mess (kind of understandable since there are so many heuristics), but still. I'm not sure why we detect links, then throw away the line info, then detect links again and try to match them back up. Weird.

Another thing is that you have the column number of the links in both cases so we should probably check if the passed link actually overlaps the detected link. But I haven't done that.

To reproduce the issue and check the fix, make a directory containing these two files:

❯ cat output.txt 
Warning: No default value src/main.sail:5.5-9:
Warning: No default value src/main.sail:5:

❯ cat sub/src/main.sail 
a00000000000000000000000000
b00000000000000000000000000
c00000000000000000000000000
d00000000000000000000000000
e00000000000000000000000000
f00000000000000000000000000
g00000000000000000000000000
h00000000000000000000000000

Now ctrl-click the two src/main.sail links from cat output.txt. Before this patch only src/main.sail:5 works. The other one just copies the link text to the quick picker but doesn't open the file.

After this patch they both work.

Oh also, to be clear this did work already if the link happens to match the exact path to the file. It's only when it is in a subdirectory somewhere that you see the difference.

I'm not sure why we detect links, then throw away the line info, then detect links again and try to match them back up. Weird.

You might be referring to the TerminalLocalLinkDetector which provides validated (confirmed on the FS) for high confidence links found? When the link is not validated it'll fall back to a word/search link which makes a link out of every word (on hover) and activating it will do the heavier work (eg. pinging the file system which might be on a remote machine) which may involve re-checking the links.

Allowing word links to read col/line context at all was actually a very recent change: https://code.visualstudio.com/updates/v1_88#_additional-context-for-word-links

Ah that kind of makes sense, though it still feels a little round about. So for my example use case we have:

1. TerminalLocalLinkDetector finds file links, including line/column numbers. It will find { path: "src/main.sail", line: 5, col: 5 } (roughly). It only returns these if they exactly match a file on disk. In this case it doesn't match because the file is in a subdirectory.
2. It falls back to treating src/main.sail:5.5-9 as a "word" and tries to open that link.
3. We run link detection on the line again, and re-find { path: "src/main.sail", line: 5, col: 5 }. After the hacky manipulation in this patch we say "oh it's the same src/main.sail; lets use that instead".
4. It gets sent to the quick picker which is able to its file database to find the file.

I think in an ideal world if TerminalLocalLinkDetector finds a link it's still preferable to use that link rather than falling back to word-based links even if the file doesn't exist on disk? But... let's not do that now! :-D

@Timmmm you should have seen it before I reworked it into the detectors and openers sub-systems 😉. Doing it this way allows loose coupling and easier testing while keeping the flexibility. The cost of potentially reparsing a single link on activate is fairly low so the extra parse doesn't concern me that much. The more important thing is keeping the flexibility of the current system where there are n detectors each with their own priority can create links of any opener type.

Created an issue for this bug #216728

Thanks!