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

Jump to bottom

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
FrozenDueToAge gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone
Unreleased

Comments

@muirdm
Copy link

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 This label describes issues relating to any tools in the x/tools repository. label Dec 5, 2019
@gopherbot
Copy link

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 Issues related to the Go language server, gopls. label Dec 5, 2019
@stamblerre
Copy link
Contributor

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
Copy link

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

@stamblerre
Copy link
Contributor

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
Copy link
Author

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.

@golang golang locked and limited conversation to collaborators Dec 5, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
3 participants
@stamblerre @gopherbot @muirdm