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: move "go.import.add" from vscode-go to gopls #43351

Open
marwan-at-work opened this issue Dec 23, 2020 · 5 comments
Open

x/tools/gopls: move "go.import.add" from vscode-go to gopls #43351

marwan-at-work opened this issue Dec 23, 2020 · 5 comments
Labels
gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone
gopls/unplanned

Comments

@marwan-at-work
Copy link
Contributor

marwan-at-work commented Dec 23, 2020 •
edited

Now that gopls will be on by default, vscode should delegate its remaining non-lsp functionality to gopls through nonStandardRequest, if said feature makes sense to be moved there.

Starting with the "go.add.imports" command as an example, the "Add Import" action should be a good first migration and it has a number of benefits:

  1. The logic is straightforward and most of it already exists in gopls
  2. The vscode-go implementation is currently using regex to parse an import statement and always adds imports to the top of the line, while gopls can more nicely add an import line in the correct place within an import block.
  3. gopls can filter the list in a much smarter way than the client such as: filtering out already-imported packages, filtering out un-importable internal packages and cyclic imports.
  4. The existence of this method in gopls will also open the door to other editors to use it since some of these functionalities are quite Go specific and are likely not going to make it into the LSP spec.

Once this logic is both in vscode and gopls, more custom features can be proposed to be added to gopls such as "listing all interfaces in a workspace" and "picking an interface to implement" without having to wait for a diagnostic error, etc.

If this looks okay to everyone, I have a couple of working branches for both repos I'd be happy to contribute: here and here

Thanks!
@marwan-at-work marwan-at-work added the gopls Issues related to the Go language server, gopls. label Dec 23, 2020
@gopherbot
Copy link

Change https://golang.org/cl/281412 mentions this issue: internal/lsp: add knownPackages and addImport as nonStandardRequests

@myitcv
Copy link
Member

myitcv commented Jan 5, 2021

This was previously discussed, but I'm having a hard time finding the issue at the moment!

@stamblerre
Copy link
Contributor

I believe the original issue is #32749.

@myitcv
Copy link
Member

myitcv commented Jan 5, 2021

Thanks. For some reason I was only searching closed issues!

@stamblerre stamblerre added this to the gopls/unplanned milestone Jan 31, 2021
gopherbot pushed a commit to golang/tools that referenced this issue May 23, 2021
@marwan-at-work @stamblerre
internal/lsp: add list_known_packages and add_import commands
4061312 
This CL adds two new commands that let a client request a list of importable packages relative to a Go file and then select which import a programmer would like to add to said file.

Updates golang/go#43351

Change-Id: If12518874a92ed4167bdd711a92e03ee21c7b949
Reviewed-on: https://go-review.googlesource.com/c/tools/+/281412
Trust: Rebecca Stambler <rstambler@golang.org>
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
@hyangah
Copy link
Contributor

hyangah commented Nov 2, 2023

I was about to file a quality issue about the quality of gopls.list_known_packages and remembered this issue.

VS Code Go's "go.import.add" (aka "Go: Add Import") was migrated to use gopls.list_known_packages a while ago. Should we continue the quality improvement discussion in this thread, or should we open another issue (because the goal for this issue is already achieved)? @findleyr

Currently gopls.list_known_packages returns results in lexicographical order which is not very useful. And it also includes unimportable packages (e.g. internal/* and vendor/*). The list can be also quite long (I think #32749 may help here).

There are also #32749 and #48545.
They are similar but not exactly capture my feature request.

@hyangah hyangah changed the title gopls: Move "go.add.imports" from vscode-go to gopls gopls: move "go.import.add" from vscode-go to gopls Nov 2, 2023
@seankhliao seankhliao changed the title gopls: move "go.import.add" from vscode-go to gopls x/tools/gopls: move "go.import.add" from vscode-go to gopls Nov 2, 2023
@gopherbot gopherbot added the Tools This label describes issues relating to any tools in the x/tools repository. label Nov 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
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
gopls/unplanned
Development

No branches or pull requests
5 participants
@myitcv @hyangah @stamblerre @gopherbot @marwan-at-work