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: add support for more code lenses #36787

Open
stamblerre opened this issue Jan 27, 2020 · 15 comments
Open

x/tools/gopls: add support for more code lenses #36787

stamblerre opened this issue Jan 27, 2020 · 15 comments

Comments

@stamblerre
Copy link
Contributor

@stamblerre stamblerre commented Jan 27, 2020

We should support the textDocument/codeLens request type.
The VS Code Go extension has a number of code lenses we could add as a starting point.
See microsoft/vscode-go#3003.

@stamblerre
Copy link
Contributor Author

@stamblerre stamblerre commented Feb 7, 2020

@hyangah suggested that we could use code lenses to offer the user an option to start a local godoc server.

@hyangah
Copy link
Contributor

@hyangah hyangah commented Apr 2, 2020

Another code lens feature VS Code Go provides is 'run test' and 'debug test' code actions. They are currently through a separate GoDocumentSymbolProvider that invokes go-outliner underneath. Would be nice if gopls provides the code action as well.

@martskins
Copy link

@martskins martskins commented May 5, 2020

I'm keen to give the run test code lens a try. I currently have a basic implementation that uses a command (much like the go generate code lens does) to run the test.
The question now is what we want to do with the output of that command, as it stands now I just call ShowMessage with test passed or test failed based on the output of the command, but maybe we want to show the full output of the test somewhere (if it fails). Any ideas about this last point?

@stamblerre
Copy link
Contributor Author

@stamblerre stamblerre commented May 5, 2020

That's awesome @martskins - please do send a changelist when you're ready :) I think the show message is fine for now (we do something similar for go generate here). You can also use the progress reporting, which I believe gives access to the output channel.

Ideally, we'd be able to write to the VS Code output channel like the extension does, but LSP doesn't support that yet. Looks like @marwan-at-work, who wrote the go generate code lens, has already filed that request: microsoft/language-server-protocol#945.

@marwan-at-work
Copy link
Contributor

@marwan-at-work marwan-at-work commented May 5, 2020

Running tests is tricker than running go generate because if the tests fail, you really wanna look at why a test fails (especially if you're running table tests or package-level tests, you wanna know which test exactly failed)

In go generate we are using the WorkDoneProgress UI (which only shows one log at a time and disappears right away) so the tests won't work for that.

go generate however falls back to the gopls window-output if WorkDoneProgress is not supported by the editor.

The problem with the gopls window-output is twofold:

  1. You can't have the LSP tell the Client to "show the window now" -- so the user must know how to find it.

  2. That window is cluttered with LSP specific logs (for a good reason ) and even the go-generate output is actually specially formatted for the LSP Inspector.

So from VSCode's perspective, using that will actually be a much less friendly user experience than what's currently in place until the proposal Rebecca mentioned is accepted and implemented in clients.

I believe it was mentioned to me at some point that you can do custom requests and each client will have to implement that custom request, so that's another route but im not familiar with it :)

@hyangah
Copy link
Contributor

@hyangah hyangah commented May 5, 2020

For VS Code, there is also 'debug test' code lens that works closely with the debugger. I expect that will be hard to delegate to gopls. For VS Code, we will probably end up using this as a hint to detect 'Test*', 'Benchmark*' functions and test command arguments. Then, translate it back to our own VS Code commands from lsp middleware, instead of delegating the test run to gopls.

@martskins
Copy link

@martskins martskins commented May 5, 2020

As a vim user I would say that I would find it useful to be able to run tests delegating it to gopls, even if it doesn't output the failure message. I understand that this is far from ideal though. But being able to at least check that the tests still pass is a pretty decent addition.

The idea to use VS Code (or vim, or you name it) itself to run the tests sounds to me as a decent compromise until that proposal is accepted though.

@marwan-at-work
Copy link
Contributor

@marwan-at-work marwan-at-work commented May 5, 2020

@martskins in VSCode, we already have this implemented on the plugin-level and it uses output channels appropriately. So it would be a step backwards in terms of user experience in VSCode.

Would it be possible for gopls to toggle this on/off so that vim users can turn it on and VSCode users can turn it off? Preferably automagically 😄

@stamblerre
Copy link
Contributor Author

@stamblerre stamblerre commented May 5, 2020

@martskins in VSCode, we already have this implemented on the plugin-level and it uses output channels appropriately. So it would be a step backwards in terms of user experience in VSCode.

Would it be possible for gopls to toggle this on/off so that vim users can turn it on and VSCode users can turn it off? Preferably automagically 😄

Yeah, the plan is to have this feature be off by default until it reaches parity with VS Code. (See more detail in @findleyr's comment on https://golang.org/cl/231959).

@heschik
Copy link
Contributor

@heschik heschik commented Jun 16, 2020

We have many lenses now. What's left to do?

@stamblerre
Copy link
Contributor Author

@stamblerre stamblerre commented Jun 16, 2020

I would say that the remaining work is to check which code lenses VS Code supports, so that we can try to reach feature parity. Also, we could add the code lens @hyangah suggested in #36787 (comment).

@hyangah
Copy link
Contributor

@hyangah hyangah commented Jun 24, 2020

vscode-go plans to utilize gopls's codelens by replacing the commands
with vscode-go native commands, from our language client middleware.

List of codelenses currently offered by vscode-go. (@mcjcloud)

  • ‘run package tests’ -> ‘go.test.package’
  • ‘run file tests’ -> ‘go.test.file’
  • ‘run package benchmarks’ -> ‘go.benchmark.package’
  • ‘run file benchmarks’ -> ‘go.benchmark.file’
  • ‘run test’ -> ‘go.test.cursor’
  • ‘debug test’ -> ‘go.debug.cursor’
  • ‘run benchmark’ -> ‘go.benchmark.cursor’. (cl/243159)
  • ‘debug benchmark’ -> ‘go.debug.cursor’

Codelenses with open feature requests

@stamblerre stamblerre changed the title x/tools/gopls: support code lenses x/tools/gopls: add support for more code lenses Jul 23, 2020
@stamblerre stamblerre removed this from the Unreleased milestone Jul 23, 2020
@gopherbot
Copy link

@gopherbot gopherbot commented Jul 28, 2020

Change https://golang.org/cl/245358 mentions this issue: src/goLanguageServer.ts: add "debug test" and "debug benchmark" codelens

@mcjcloud
Copy link

@mcjcloud mcjcloud commented Jul 28, 2020

vscode-go plans to utilize gopls's codelens by replacing the commands
with vscode-go native commands, from our language client middleware.

List of codelenses currently offered by vscode-go. (@mcjcloud)

  • ‘run package tests’ -> ‘go.test.package’
  • ‘run file tests’ -> ‘go.test.file’
  • ‘run package benchmarks’ -> ‘go.benchmark.package’
  • ‘run file benchmarks’ -> ‘go.benchmark.file’
  • ‘run test’ -> ‘go.test.cursor’
  • ‘debug test’ -> ‘go.debug.cursor’
  • ‘run benchmark’ -> ‘go.benchmark.cursor’. (cl/243159)
  • ‘debug benchmark’ -> ‘go.debug.cursor’

Codelenses with open feature requests

The "debug test" and "debug benchmark" code lens will be available with gopls enabled as of https://golang.org/cl/245358.

gopherbot pushed a commit to golang/vscode-go that referenced this issue Jul 29, 2020
This CL adds a "debug test" and "debug benchmark" codelens to the
intercepted lenses from gopls.

Additionally, a fix is included for finding the function name argument
in the arguments that come from gopls.

Updates golang/go#36787

Change-Id: I1a109902bbee25ecb9bbf4ae1c8d9af1b6d2fa2f
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/245358
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
TryBot-Result: kokoro <noreply+kokoro@google.com>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
@gopherbot
Copy link

@gopherbot gopherbot commented Jul 30, 2020

Change https://golang.org/cl/246017 mentions this issue: lsp/code_lens.go: add "run file bencharks" code lens

gopherbot pushed a commit to golang/tools that referenced this issue Aug 12, 2020
This CL adds a code lens to run all benchmarks in a file. Additionally,
it updates the test command handler to better support both tests and
benchmarks.

Updates golang/go#36787

Change-Id: I6e90460f7d97607f96c263be0754537764bd0052
Reviewed-on: https://go-review.googlesource.com/c/tools/+/246017
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
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
7 participants
You can’t perform that action at this time.