Show "Go to Implementation" in the Context Menu #54317
Conversation
Go to/Peek definition and Go to Type definition had order 1.1, 1.2, 1.4, so it seems that this was not included by mistake.
|
cc @mattbierner |
|
We removed from the menu as it was not seen as an common enough operation to justify its own context menu entry. #9285 tracks making these menus user configurable |
|
That may be very anecdotal evidence, but it seems like it should be a very common operation for popular OOP languages like C++, Java, C#. Making it configurable definitely sounds like a good idea - then again, it’d be nice to provide a unified mechanism for every extension (that might be the case, I’m not yet familiar with how customizable the design might be). After all, if a language doesn’t support such operation (doesn’t provide an active ImplementationProvider), then this wouldn’t be even on the context menu. I’ll ask for mentoring instructions at the linked issue, since it’d be nice to have this implemented. cc @dbaeumer from the LSP side of things |
|
Looking forward for this functionality. A lot of people I know would love to have this. |
|
Any updates on this feature? |
|
We didn't add this to the context menu because we believe this is not a common enough operation. We have looked at the way people use editors/IDEs when we made that decision. Please take into account that adding something to the context menus for everybody is nothing that we want to do in a rush and only if it is absolutely necessary. The reasoning is that once added, it is almost impossible to remove something without infuriating thousands of users. And if we add things, we will soon have to build scrollbars for our context menus like other tools, which is ... undesirable. I honestly believe the best way forward is to implement #9285, such that each user can customise context menus to their liking. Since context menus are built on top of context keys (similar to keybindings), it should be somewhat straight-forward to expose it and make it configurable... |
This was used before LSP has considered (and now stabilised) a 'go to implementation' request. Handling this via LSP, even at the cost of command disappearing from the context menu [1], is preferred than using a global command. We'd have to maintain an extra mapping and guess which RLS server is responsible for the file for which a request has been made. This is hacky and not guaranteed to be correct; meanwhile we get errors in multi-root workspaces whenever we open more than one folder due to each instance trying to register a global command under the same identifier. The clean solution is to remove this extra command and use the standard Go to Implementation (under Go > Go to Implementation [Ctrl + F12]) while we wait for the user configurable menus [2] to land. [1]: microsoft/vscode#54317 [2]: microsoft/vscode#9285
|
This has been done in the meantime:
|
When initally implemented in #18346, Go to Implementation was listed on the menu, after which it was replaced there with #19699 in favor of Go to Type Definition.
Since the LSP stabilised in 3.6 the Go to Implementation alongside Go to Type Definition, I believe we should bring it back to the menu to help discover the feature.
It's only populated in the menu only when there's an implementation provider for current document selector, so overpopulating the menu shouldn't be a problem.
I know that Rust folks should be happy with this addition (being able to navigate via trait implementations is very helpful, in fact we're just switching from using our own custom 'find impls' command/menu item to LSP 3.6 in rust-lang/rls#936), but I believe the same can be said for other devs (e.g. Java).