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

With multiple definitions "Go to definition" shows peek definition popup instead of going to definition #70032

Closed
selrond opened this issue Mar 8, 2019 · 11 comments
Assignees
Labels
editor-symbols definitions, declarations, references feature-request Request for new features or functionality on-testplan under-discussion Issue is under discussion for relevance, priority, approach
Milestone

Comments

@selrond
Copy link
Contributor

selrond commented Mar 8, 2019

  • VSCode Version: 1.32.1 (1.32.1)
  • OS Version: 10.14.3 (18D109)

Steps to Reproduce:

  • happens sometimes when I work with JS files. I couldn't find a pattern in when it works and when it does not... It's extremely frustrating

Does this issue occur when all extensions are disabled?: Yes

@vscodebot vscodebot bot added the new release label Mar 8, 2019
@alexr00
Copy link
Member

alexr00 commented Mar 8, 2019

Also happening on insiders.

@Ky6uk
Copy link

Ky6uk commented Mar 8, 2019

It happens for TS files as well. A click to Go to Definition from context menu also shows definitions popup.

@jrieken
Copy link
Member

jrieken commented Mar 10, 2019

That's the outcome of #68023. When having multiple definition of a symbol the following change was made:

  • before: open the nearest symbol and show peek at its position
  • after: show peek at position of request

So, this happens whenever multiple definitions of a symbol exist. Instead of making a decision where to go to first, VS Code now shows peeks without revealing one. After reading/hearing feedback I was under the assumption that is wanted but we can consider an options if people feel strong about restoring the previous, jump to nearest, definition behaviour.

@jrieken jrieken added under-discussion Issue is under discussion for relevance, priority, approach editor-symbols definitions, declarations, references labels Mar 10, 2019
@jrieken jrieken changed the title Go to definition shows peek definition popup instead of going to definition With multiple definition "Go to definition" shows peek definition popup instead of going to definition Mar 10, 2019
@jrieken jrieken changed the title With multiple definition "Go to definition" shows peek definition popup instead of going to definition With multiple definitions "Go to definition" shows peek definition popup instead of going to definition Mar 10, 2019
@Ky6uk
Copy link

Ky6uk commented Mar 10, 2019

"Go to" should work as "go to" I guess. Jumping to a nearest definition was the well-worked thing for me. Right now I always need to one additional mouse move + click every time I'm navigating to definitions. That's happening very often because jumping, for example, to any styled component in react application with types from DT always have two references, one reference is mine and one reference from DT:

Screenshot 2019-03-10 at 11 45 10

Obviously the second one isn't interesting for me. So please add the option to restore the previous behaviour. Or something like Go to closest definition.

@aminpaks
Copy link

aminpaks commented Mar 10, 2019

Shouldn't it go to the definition of the variable?
if I have declared a function/variable and exported, it seems going to the declaration make more sense no?

Also what's the difference between "Goto Definition" and "Pick Definition" now, it seems they both do the same thing.

@nonoroazoro
Copy link

IMO, these changes are making VSCode less convenient.

@jrieken
Copy link
Member

jrieken commented Mar 11, 2019

Also what's the difference between "Goto Definition" and "Pick Definition" now, it seems they both do the same thing.

"Peek Definition" always shows the peek window, "Go To Definition" shows it when having multiple definitions. We didn't change that, what we have changed is that peek opens at the position of the request, not at the position of the first definition.

I will add a setting that allows to restore the old behaviour, to workaround until then, hit Enter and the the first definition will be revealed.

@selrond
Copy link
Contributor Author

selrond commented Mar 11, 2019

@jrieken I don't get the

"Go To Definition" shows it when having multiple definitions

part...

I don't see how a symbol (an import in JS in my case) can have multiple definitons.

for example:

export { Cart: createIcon(Cart) }

where createIcon is a HOC, that takes imported svg React component from elsewhere, and wraps it in some wrappers.

It seems to me that Cart's go to definition would take me to where Cart is defined, but instead this shows up:

image

@jrieken
Copy link
Member

jrieken commented Mar 11, 2019

I don't see how a symbol (an import in JS in my case) can have multiple definitons.

Let's keep that out of this issue. Generally, a symbol can have multiple definitions and this issue is about the UI for that case, e.g where to show peek for multiple symbols. If you disagree with how TypeScript computes definitions for a certain symbol and if you think it should be 1, not more, then we should move that into a separate issue.

jrieken added a commit that referenced this issue Mar 11, 2019
@jrieken
Copy link
Member

jrieken commented Mar 11, 2019

With next Insiders build (http://code.visualstudio.com/insiders/, tomorrow or sooner) there will be a new setting: editor.gotoLocation.many which controls what happens when encountering multiple definition/declarations of a symbols. Possible values are

  • 'peek' (default) - Open peek with all results at the position of the request
  • 'revealAndPeek' - Reveal the first result and open peek with all results there
  • 'reveal' - Reveal the first result and ignore others

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
editor-symbols definitions, declarations, references feature-request Request for new features or functionality on-testplan under-discussion Issue is under discussion for relevance, priority, approach
Projects
None yet
Development

No branches or pull requests

6 participants