Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature request: "Prefer Go To Implementation" #3612

Open
cdbattags opened this issue Nov 22, 2024 · 4 comments
Open

Feature request: "Prefer Go To Implementation" #3612

cdbattags opened this issue Nov 22, 2024 · 4 comments
Labels
WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Milestone

Comments

@cdbattags
Copy link

Is your feature request related to a problem? Please describe.
Using the cmd+click/option+click flow, I always go to the interface instead of going to the source for the implementation.

Describe the solution you'd like
I would like a setting that opens the "Go to Implementations" dialog instead or might even go to the first implementation automatically.

Describe alternatives you've considered
https://github.com/microsoft/vscode/blob/e1de2a458dfb770545489daf499131fd328924e7/extensions/typescript-language-features/package.json#L1485 as a reference for how TypeScript extension deals with this. I've tried to see if I could add this behavior myself but for now I just rebound the cmd+f12 keybind instead.

Additional context
N/A

@cdbattags cdbattags changed the title I'd like a way to "Prefer Go To Implementation" Feature request: "Prefer Go To Implementation" Nov 22, 2024
@gopherbot gopherbot added this to the Untriaged milestone Nov 22, 2024
@adonovan
Copy link
Member

I'm a little confused by this request, because the default behavior of VS Code + Go is that the context menu always offers one item called "Go to Implementations", and the actual operation that this performs (as determined by gopls) depends on whether the selected type is a concrete type or an interface type. For concrete types, it shows interfaces, and for interface types, it shows concrete implementations.

This means that there is no way to ask for more specific interfaces that implement a given interface (see microsoft/language-server-protocol#2037). Is that what you are asking for?

@adonovan adonovan added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Nov 25, 2024
@firelizzard18
Copy link
Contributor

I believe the request is, As a user when I hover/select a method call of an interface value and I execute Go To Definition I want it to jump to the implementation of that interface method (if configured to do so).

If there are multiple implementations, clearly we can't jump since the destination is undefined. However, if the workspace only contains a single implementation of the interface, jumping to that implementation of that method is feasible. That being said, when I execute Go To Definition in JavaScript/TypeScript, sometimes it pops up a preview/hint that includes multiple results. So we should be able to do something similar and list all of the implementations when the user executes Go To Definition on an interface method with multiple implementations.

@adonovan
Copy link
Member

I believe the request is, As a user when I hover/select a method call of an interface value and I execute Go To Definition I want it to jump to the implementation of that interface method (if configured to do so).

When an interface method call is selected, "Go to Definition" jumps to the definition of the symbol (in this case, an interface method) whereas "Go to Implementations" shows all the corresponding concrete methods. The first operation is strictly about the reference graph (which relates references to declarations), which is inherently a "downwards" operation in terms of the import graph; that is, the interface is always among the dependencies of the current package. The second operation is a workspace-wide query for matching methods; they could be anywhere at all.

I think it would be extremely confusing for users if we sometimes changed the meaning of "Go to Definition" to mean "Go to implementations". Why not just invoke the latter operation? They are sibling items on the same menu. Perhaps I am missing something.

@firelizzard18
Copy link
Contributor

Why not just invoke the latter operation? They are sibling items on the same menu.

The majority of the time when I execute Go To Definition on a method it's because I want to see the source of that method. I don't open the menu, I control-click on the method. The reason this feature would be useful to me comes down to workflow. I'm jumping around through the code and I want to quickly jump to the implementation of a method, either to edit it or to read and understand it. Hitting this case is jarring. Generally what happens is I control-click the method, which jumps to the interface, so then I have to right click and execute Go To Implementations then jump to the implementation. Mentally I'm in the mode of reading code so when I control-click I expect to see the implementation, and having to switch gears breaks that flow. I could check each time if the receiver is an interface value, but that still breaks my flow. If the interface has multiple implementations I'm not certain whether my suggestion (show the preview window thing) is valuable, but for me personally Go To Definition jumping to the implementation if there's only one would be valuable more often than not.

I think it would be extremely confusing for users if we sometimes changed the meaning of "Go to Definition" to mean "Go to implementations"

Hence the request for an option. If I opt-in to this behavior I'm far less likely to find it confusing, and it would be actively beneficial to me in most cases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Projects
None yet
Development

No branches or pull requests

4 participants