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: create a new snapshot on didChangeConfiguration #42814

Open
stamblerre opened this issue Nov 24, 2020 · 3 comments
Open

x/tools/gopls: create a new snapshot on didChangeConfiguration #42814

stamblerre opened this issue Nov 24, 2020 · 3 comments

Comments

@stamblerre
Copy link
Contributor

@stamblerre stamblerre commented Nov 24, 2020

It would probably simplify our logic to treat this as an invalidation event, especially since a change in environment leads to metadata invalidations.

@stamblerre stamblerre added this to the gopls/unplanned milestone Nov 24, 2020
@stamblerre stamblerre removed this from the gopls/unplanned milestone Dec 18, 2020
@stamblerre stamblerre added this to the gopls/v1.0.0 milestone Dec 18, 2020
@stamblerre stamblerre added this to To Do in gopls on-deck Feb 28, 2021
@krobelus
Copy link

@krobelus krobelus commented Aug 8, 2021

I'm not sure if this is the same issue: I expect that settings sent via didChangeConfiguration are applied immediately

For example, given this file

package main

func main() {
	println()

}
  1. I start goplswith no configuration
  2. I send {"jsonrpc":"2.0","method":"workspace/didChangeConfiguration","params":{"settings":{"formatting.gofumpt":true}}}
  3. I send {"jsonrpc":"2.0","method":"textDocument/formatting", ...}
  4. Nothing happens; I expect the empty line after pringln() to go away.

It works when I restart the language server, or when I use workspace/configuration instead.
I don't think a restart should be necessary, and not all clients (want to) implement workspace/configuration. For example it doesn't really make sense for the client I'm using - https://github.com/kak-lsp/kak-lsp - some implementation reasons prevent mult-workspace-in-single-editor-session for now.


Tangentially related: my main problem with workspace/configuration is that it needs section headers. To send "formatting.gofumpt", the user needs to specify {"gopls":"formatting.gofumpt"}. OTOH "formatting.gofumpt" already works when sent as initialization option. I'd like to use a single config object for both, so the user doesn't have to care about protocol details. In the config system of kak-lsp, there is one object for each language, so I'm having a hard time understanding the need for the section header.

@stamblerre
Copy link
Contributor Author

@stamblerre stamblerre commented Aug 9, 2021

@krobelus: Your topic is actually a separate issue, but I can answer it briefly. For some reason, VS Code doesn't actually send the modified settings in its didChangeConfiguration request, so we rely on workspace/configuration to get the updated values. We also use workspace/configuration to get the settings at initialization.

You shouldn't need section headers for workspace/configuration. Here is an example request/response from VS Code:

[Trace - 17:08:47.027 PM] Received request 'workspace/configuration - (2)'.
Params: {"items":[{"scopeUri":"file:///Users/rstambler/code/src/golang.org/x/tools","section":"gopls"}]}


[Trace - 17:08:47.033 PM] Sending response 'workspace/configuration - (2)' in 5ms.
Result: [{"verboseOutput":true,"ui.diagnostic.analyses":{"fillreturns":true},"ui.semanticTokens":true,"experimentalUseInvalidMetadata":true,"ui.diagnostic.staticcheck":true,"ui.completion.experimentalPostfixCompletions":true,"ui.completion.usePlaceholders":true,"build.experimentalTemplateSupport":true,"build.experimentalWorkspaceModule":true}]

@krobelus
Copy link

@krobelus krobelus commented Aug 10, 2021

Thanks that's good to know. It looks like gopls accepts the same options in both didChangeConfiguration and workspace/configuration which makes things simpler, but other servers might not.

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

Successfully merging a pull request may close this issue.

None yet
3 participants