-
Notifications
You must be signed in to change notification settings - Fork 30.2k
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
Go to Definition shouldn't open file when there are multiple definitions #31046
Comments
There are two separate features: "Peek Definition" and "Go to Definition". Ctrl+click is bound to the latter, not former. "Peek Definition" doesn't change the current location but shows all definitions of a symbol in-place. "Go to Definition" does change the current location, 'go to' vs 'peek'. In case of multiple definitions it jumps to the nearest definition, showing all definitions of that symbol in a peek view. |
Ah I see, sorry I didn't know the terminology. So to more succinctly describe the issue: "Go to Definition" should act exactly like "Peek Definition" does when there is more than one definition. Can you reopen this? By the way if someone gives me a pointer to the code that controls this I might have a go at a patch. |
The design was a conscious decision and I am not ready to change the behaviour because of a single feedback item. That means there are no plans to change the behaviour unless a large enough group of people vote for this. |
Ah ok. To be honest I think it's obvious that that should be the behaviour. I asked on StackOverflow a while ago how to entirely disable it instead of improving it but I thought that would go down less well here. Seems like at least 9 other people find it confusing so far. Also there are a couple of other very highly upvoted questions asking how to disable peek in Visual Studio proper, so I think it is not a particularly loved UI. You can turn it off in Visual Studio but I'd be happy if it just did the logical thing at least in VSCode. |
I agree with @Timmmm. If there are multiple definitions, it makes no sense for it to jump to a single definition, without giving the user the option of selecting which one. Sublime Text opens the navigator with the files containing the definition, so it's easy to select which one you want to open. |
This seems to have been fixed in a newer version. If I now hover over |
This does not seemed to be fixed in Version 1.24.1. When I ctrl+click on a method I am taken to the method with a list of all other methods with the same name. I would also prefer to stay in place and be presented with a list of options to choose. Furthermore, this feature also shows a list of all methods with the same name in all classes. This seems incorrect to me since the method is scoped to the class that contains it and it is not the same as any other methods in other classes. Yet I am presented with hundreds of options because they have the same name in other classes. There should be no peek in this situation. |
The C/C++ extension would prefer Go to Definition not auto-goto the 1st definition found, since it typically wouldn't be the one desired so remaining at the same location is preferred -- users generally don't know beforehand if multiple definitions will be found or not. Some sort of opt-in setting would be fine (not sure what other languages have this problem). When our extension implements an improved go to definition which can actually find the correct (single) definition, then this won't be as much of an issue unless it falls back to the old behavior due to setup issues. |
@johanforssell If one of the Go to Definition targets is invalid, I believe you should create a new bug...otherwise, I think it's "by design". This issue is just about the fact that it auto-moves the context to one of the definitions. |
I also find the current behavior annoying. How do I vote, @jrieken? |
I would like to add, right now for multiple definitions it goes to the first one in alphabetical order based on file names. It would be nice to be smarter than that. For example prefer the actual definition instead of a forward declaration. Maybe be even smarter and combine all the forward declarations into one tree unit. And for less than a sane number of results like 20, just expand everything automatically. No reason to have 5 results in 5 different files all collapsed. For more than 20 expand good looking results automatically. |
@graph The C++ extension is supposed to return definitions and exclude declarations unless no definitions are found (unless we have some bug), or maybe you're using a different definition provider. +1 on auto-expanding the results under each file. |
Sorry I meant declarations everywhere I said definitions (I think, I always get these 2 terms mixed up). E.g. in opencv if you cmd+click some opencv structs it jumps to a forward declaration of the struct first which doesn't tell me what the fields it has, I have to click a few times to find the real declaration. I'm using the default plugins/providers I have no minimal custom setup with vs code. |
If I click "Peek Declaration (Ctrl+Alt+F12)" from the context menu it open the peek view without changing file. But using Ctrl+Click it does change the file, which is a bit annoying. |
This has hopefully been fixed by #68023! |
Steps to Reproduce:
FOO
infoo.cpp
VSC realises that FOO has multiple definitions and opens an interface to let you choose which one to go to. The problem is it does that after opening
foo.h
and jumping to a random#define FOO
. It's really confusing. Look at this screenshot! I barely understand what is going on in this UI, and this is a simple example! Imagine actual code...Wouldn't this make much more sense (excuse my poor image editing)?
This happens in all languages as far as I can tell.
The text was updated successfully, but these errors were encountered: