ctrl + click "go to definition" clashes with ctrl + click "follow link"

[karrtikr]

@AngusWR commented on Wed Sep 25 2019

Environment data

• VS Code version: Version: 1.38.1
• Extension version (available under the Extensions sidebar): Version: 2019.9.34911
• OS and version: Windows 10 Pro 1903 18362.356
• Python version (& distribution if applicable, e.g. Anaconda): 3.6.7

Expected behaviour

ctrl + click on a string representation of a url opens the url in chrome

Actual behaviour

ctrl + click on a string representation of a url opens the url in chrome and also goes to the definition for str in builtins.pyi

Holding ctrl and hovering the url:

After clicking the url

I haven't been able to find a way of changing the key binding for the "go to definition" feature. Any advice?

---

@karrtikr commented on Thu Sep 26 2019

For changing keybindings, go to File -> Preferences -> Keyboard shortcuts. Search for Go to definition. Anyways, this is controlled by VSCode, not the extension. So I am transferring the issue.

[AngusWR]

@karrtikr Sorry, I should have been clearer in my original post. There is no key binding for ctrl + click that I can see anywhere. I thought this was the python extension because of this, and the fact that if I have a .js file open and ctrl + click a string url it opens normally in a new chrome tab without going to the definition. I've just noticed that ctrl + click on anything else in .js does go to the definition, so it seems you're right and this is an issue with VSCode.

Thanks for the transfer.

[jrieken]

Yeah, that sounds reasonable but we didn't anticipate that a string literal, e.g 'http://foo.com' would ever have a definition in a programming language. @karrtikr Is your extension handling pyi files? Is it the design to work like that?

[karrtikr]

AFAIK our extension by itself doesn't handle .pyi files. It looks like it's coming from jedi package, so tagging the author. cc @davidhalter Anyhow if such definitions do exist, it needs to be handled.

[davidhalter]

In a sense there is a definition of the string, which is the string itself. I understand that this is a bit confusing for users of the IDE, but Jedi is primarily a tool that exists for multiple use cases. However, I feel like goto_assignments does indeed the wrong thing here.

goto_definitions might be right, because it is actually not a "goto" function, but type inference. However it's even questionable there.

So feel free to create an issue in the Jedi issue tracker.

This is essentially the problem:

>>> import jedi
>>> jedi.Script("'asdf'", column=3).goto_definitions()
[<Definition full_name='builtins.str', description='instance str'>]
>>> jedi.Script("'asdf'", column=3).goto_assignments()
[<Definition full_name='builtins.str', description='instance str'>]

[jrieken]

Adding @alexandrudima who as written links and clickLinkGesture. I believe that there is a preventDefault (or some other way to consume the event) missing and therefore both handler (links and go-to-def-with-mouse) retrieve/handle the event

[desprit]

I experience this issue as well. Disabling Python extension stops that behavior. Unfortunately, they closed this issue: https://github.com/microsoft/vscode-python/issues/7591

[noxasch]

experiencing this issue, when i cmd+click a url it open builtins.pyi definition. any workaround ?

[stamblerre]

We've also encountered this issue, specifically with developing the Go language server. We wanted to make import statements (like import "fmt") link to documentation on the package, so we implemented textDocument/documentLink for import statements. We also figured that a user may not always have an Internet connection and should also be able to jump to the source code of the package, so we implemented that in textDocument/definition.

However, the result is that, if the user does a Ctrl + Click on an import statement, VS Code will both open a link and jump to the definition of the package, which is not a great user experience. Is there any way to work around this, or should we just disable one of the two behaviors?

[davidhalter]

This was fixed in Jedi 0.16.0, which was just released, so I guess you can close once you upgrade to that.

[stamblerre]

I think the general question still remains - is the correct approach to disable one of the two features? If so, it might be worth bringing up this issue on the https://github.com/microsoft/language-server-protocol repository, since this issue only arises due to VS Code's default keybindings.

[hediet]

This is where clicks on definitions are handled: https://github.com/microsoft/vscode/blob/f1f645f4ccbee9d56d091b819a81d34af31be17f/src/vs/editor/contrib/gotoSymbol/link/goToDefinitionAtPosition.ts#L62

This is where click on links are handled: https://github.com/microsoft/vscode/blob/f1f645f4ccbee9d56d091b819a81d34af31be17f/src/vs/editor/contrib/links/links.ts#L297

Unfortunately, both are async and I don't know if there is any guarantee on the order in which these handlers are called. Also, the ClickLinkMouseEvent instances are different.

Because preventDefault of the underlying event does not work asynchronously, the ClickLinkMouseEvent could be extended with a method queueExclusiveHandler(handler: Promise<boolean>): void (it would need to write to the underlying event). If the handler returns false at some point, the next queued handler is invoked. If the handler returns true, the queue is cleared.

[alexdima]

I think we could tackle this by introducing an editor controller e.g. FollowLinkController which would be the only one handling the mouse up editor event. It would allow interested parties to register IFollowLinkHandlers. goToDefinitionAtPosition.ts and links.ts could register participants and they could be kicked off in parallel when the gesture occurs. Once they both resolve, the FollowLinkController can handle the cases where there is a definition available, a link available, or both available. If we ever want to, we could go so far as to show an inline picker.

[matthewbordas]

Any updates on this? It's a very painful issue when writing in Golang. I can't seem to find a workaround other than using F12 or right-click + Go to Definition, both of which are slower than ctrl/cmd + click. Thank you!

[ian-h-chamberlain]

I think for gopls users, this is about to get worse with the 0.16 release, which will support Go To Definition in doc comments. Any symbol from another package with the same name as a TLD would result in this behavior, e.g.

package main

import "github.com/stretchr/testify/mock"

// Do something with a [mock.Call].
func Something(*mock.Call) {}

Even worse, the link to http://mock.call is also confusingly positioned next to the language server's hover link to pkg.go.dev (probably a lot more useful!):

Offhand, some other obvious ones that might trigger this:

• .app
• .channel
• .cloud
• .download
• .fail

The list goes on...

---

One stopgap solution that wouldn't require rebinding / redesigning mouse shortcuts might be a configurable list of TLDs/URLs to ignore for link detection, perhaps in the same format as the Trusted Domains setting. This would at least allow users to opt-out of the link conflict for some common use cases (for example, I would probably disable mock.* to workaround the example above). As far as I can tell, there is no equivalent configuration option like this currently?

[fwcd]

Just encountered the same issue in a Swift package while ctrl-clicking a dependency URL in a Package.swift. The reason seems to be that the Swift language server, SourceKit-LSP, now also provides go-to-definition for string literals (linking to Swift.String).

[ian-h-chamberlain]

To follow up on my previous comment... I realized I had not configured "workbench.trustedDomains.promptInTrustedWorkspace": true yet. I tried it to see if that would at least prevent the browser window from popping up, but instead it just displays the confirmation dialog and the desired Go to Definition never occurs. So unfortunately I think it's actually worse than the current behavior with promptInTrustedWorkspace: false.

As far as I can tell, there is no way to declare a domain as "untrusted" (i.e. automatically cancel opening it), but I guess that might suffice for this use cases as well. I'm not sure how useful that would be in general though (maybe for e.g. #82794?), so I haven't filed a separate feature request, but if it existed it might be usable as a workaround.