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

x/tools/gopls: find references shouldn't include starting identifier #36007

Closed
muirdm opened this issue Dec 5, 2019 · 5 comments
Closed

x/tools/gopls: find references shouldn't include starting identifier #36007

muirdm opened this issue Dec 5, 2019 · 5 comments
Labels
Milestone

Comments

@muirdm
Copy link

@muirdm muirdm commented Dec 5, 2019

When you find references on an identifier, the identifier itself is returned as a reference in addition to other locations that reference it. I realized this defeats my editor's if-single-result-jump-straight-there logic since there will be two results even if your identifier is only referenced in one (other) spot.

Is there a reason to keep the starting identifier in the results?

@gopherbot gopherbot added this to the Unreleased milestone Dec 5, 2019
@gopherbot gopherbot added the Tools label Dec 5, 2019
@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Dec 5, 2019

Thank you for filing a gopls issue! Please take a look at the Troubleshooting guide, and make sure that you have provided all of the relevant information here.

@gopherbot gopherbot added the gopls label Dec 5, 2019
@stamblerre

This comment has been minimized.

Copy link
Contributor

@stamblerre stamblerre commented Dec 6, 2019

Seems that LSP has helpfully provided us with an includeDeclaration boolean, which I suppose we are ignoring: https://microsoft.github.io/language-server-protocol/specifications/specification-3-14/#textDocument_references. We should probably just respect that, and then editors can decide whether or not to show the declaration.

@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Dec 6, 2019

Change https://golang.org/cl/210238 mentions this issue: internal/lsp: respect References.IncludeDeclaration setting

@stamblerre

This comment has been minimized.

Copy link
Contributor

@stamblerre stamblerre commented Dec 6, 2019

Actually, after taking a closer look - it seems like we mostly do respect this setting, unless the identifier was implicitly declared (which I believe only happens in a type switch). So, then I think this sounds like a client-side issue?

@muirdm

This comment has been minimized.

Copy link
Author

@muirdm muirdm commented Dec 6, 2019

I checked and my editor does send "includeDeclaration": false.

I think the identifier itself gets included during the normal scan over Defs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.