golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
123.08k stars 17.55k forks source link

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

Open stamblerre opened 4 years ago

stamblerre commented 4 years ago

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 https://github.com/microsoft/vscode-go/issues/3003.

stamblerre commented 4 years ago

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

hyangah commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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: https://github.com/microsoft/language-server-protocol/issues/945.

marwan-at-work commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

@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 commented 4 years ago

@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).

heschi commented 4 years ago

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

stamblerre commented 4 years ago

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 https://github.com/golang/go/issues/36787#issuecomment-583623167.

hyangah commented 4 years ago

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)

Codelenses with open feature requests

gopherbot commented 4 years ago

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

mcjcloud commented 4 years ago

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’
  • [x] ‘run test’ -> ‘go.test.cursor’
  • [x] ‘debug test’ -> ‘go.debug.cursor’
  • [x] ‘run benchmark’ -> ‘go.benchmark.cursor’. (cl/243159)
  • [x] ‘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 commented 4 years ago

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

binary4cat commented 4 years ago

Can add "which interfaces are implemented" code lens? It's just like:

0 references | impl Writer
func (devNull) Write(p []byte) (int, error) {
    return len(p), nil
}
bhcleek commented 3 years ago

I'd love to see a code lense to similar to guru's freevars command.

GopherJ commented 3 years ago

rust-analyzer has already a good support on codelens, it'll be good to see it lands in gopls as it's super useful.

rust-analyzer codelens references/implementations

with codelsn info provided by rust-analyzer (calculated based on cursor position) , I can easily debug rust test item in vim using vimspector and vimspector config:

test rust-analyzer codelens test

debug rust-analyzer codelens debug with vimspector

currently it's a pain to debug go in neovim

adonovan commented 3 months ago

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

This feature will be in gopls/v0.16, using gopls' own lightweight godoc server.

bhcleek I'd love to see a code lense to similar to guru's freevars command.

This feature will be in gopls/v0.16, though as a Code Action with a web UI, not a Code Lens.

The remaining code lenses suggested here are all variations of "run/debug this main/test/benchmark". I'm going to assert that debugging is a bigger project and should have its own issues.

As for "run test/bench", we already have a code lens for that, though it's off by default because VS Code provides a superior client-side test UX. For other editors, the main obstacle to using the "run test" command is that there is no uniform way for clients to know that the progress messages from the command form a stdout/stderr stream that should be redirected to a terminal-like buffer. See discussion (in the context of Emacs+Eglot, but the ideas generalize) at https://github.com/joaotavora/eglot/discussions/1402.

Personally I find code lenses to be a very distracting UX, and we've been discussing relegating most of gopls' current lenses to code actions. For example, Toggle GC Details flips a bit in the server that causes it to display diagnostic annotations containing information about compiler optimization decisions. Why would anyone want a Toggle GC Details code lens to appear in every source file, just for the 1% percent of the time they are thinking about microoptimizations? It should be a Code Action in a menu that is there when you need it but unobtrusive when you (mostly) don't.

So I'm inclined to say that ideally, the net change in Code Lenses should be negative. not positive. Some features, such as https://github.com/golang/go/issues/56695 (if only we knew how to implement it efficiently), do perhaps make sense as code lenses, though they could also be expressed as inlay hints.

Any objections to closing this issue?

bhcleek commented 3 months ago

This feature will be in gopls/v0.16, though as a Code Action with a web UI, not a Code Lens.

What role will the web UI have? How will something like Vim on a terminal use that?

adonovan commented 3 months ago

This feature will be in gopls/v0.16, though as a Code Action with a web UI, not a Code Lens.

What role will the web UI have? How will something like Vim on a terminal use that?

There are a number of useful features we can offer though a browser that we can't realistically squeeze into LSP itself, at least not yet. In the future, I would love for LSP extensions enable us to:

The web UX isn't ideal but a large majority of users run on windowing environments (though not all of them love to switch windows). If you're running in a terminal over ssh then it may be possible to proxy the showDocument(URI) requests to a local web browser. If you're not using windowing at all (e.g. linux console), then unfortunately the web features won't be of much use.