x/tools/gopls: implement didChangeConfiguration #32258

zchee · 2019-05-26T14:50:20Z

What version of Go are you using (`go version`)?

$ go version
go version devel +385b2e0cac Fri May 24 21:34:53 2019 +0000 darwin/amd64

Does this issue reproduce with the latest release?

Might be Yes. It's related LSP protocol spec.

What operating system and processor architecture are you using (`go env`)?

go env Output

$ go env
GO111MODULE="on"
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/zchee/.cache/go-build"
GOENV="/Users/zchee/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/zchee/go"
GOPROXY="https://proxy.golang.org"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"
GCCGO="gccgo"
AR="ar"
CC="/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/clang"
CXX="/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/clang++"
CGO_ENABLED="1"
GOMOD="/Users/zchee/go/src/github.com/zchee/nvim-lsp/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/6_/10hjzx0d027_8rfblcw1lbm40000gp/T/go-build972805647=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

gopls send protocol.RegistrationParams when initialized, the ref is here:

https://github.com/golang/tools/blob/7be61e1b0e514e106d83fc439d56a79143738603/internal/lsp/general.go#L117-L125

But AFAIK, the vscode-languageserver-node/client just ignore if []Registration. RegisterOptions is empty.

vscode-languageserver-node/client.register
- https://github.com/microsoft/vscode-languageserver-node/blob/625fbdbe492f7b4b95e6d29e0adf36c01b88d056/client/src/client.ts#L790-L792
internal/lsp/protocol/Registration.RegisterOptions
- https://github.com/golang/tools/blob/7be61e1b0e514e106d83fc439d56a79143738603/internal/lsp/protocol/tsprotocol.go#L177

I also now develop Language Server Protocol client side for IDE written in Go, but I can't handle that gopls send RegisterCapability call.

What did you expect to see?

empty Registration.RegisterOptions fields RegisterCapability call.

What did you see instead?

non-nil Registration.RegisterOptions.

The text was updated successfully, but these errors were encountered:

zchee · 2019-05-26T14:50:53Z

/cc @stamblerre

stamblerre · 2019-05-28T18:33:02Z

Thanks for catching this! This means that the RegistrationOptions field of the Registration struct should be an *interface{}, not an interface{}. @pjweinb, can you make this change in the TypeScript generator?

zchee · 2019-05-30T07:45:50Z

@stamblerre
Also, the Registration.ID will use to client/unregisterCapability call.
It would be better to use unique ID such as UUID like LSP spec.

{
	"method": "client/registerCapability",
	"params": {
		"registrations": [
			{
				"id": "79eee87c-c409-4664-8102-e03263673f6f",
				"method": "textDocument/willSaveWaitUntil",
				"registerOptions": {
					"documentSelector": [
						{ "language": "javascript" }
					]
				}
			}
		]
	}
}

ianthehat · 2019-05-30T15:05:28Z

The id only has to be unique within the server (not the client) and is only used if you make an unregister call.
Given that we only register in this function, which only happens once, and we never unregister, the current values meet the requirements for uniqueness.

pjweinb · 2019-05-31T08:30:55Z

I think I do not understand. 1. In the LSP Typescript specification, RegisterOptions is an optional field with type any. The Go code (tsprotocol.go:177) has interface{} for the corresponding type, and "omitempty" for json. What should be a pointer? 2. Gopls is using the name of the method as the id in registering a capability. That seems like a good thing to use if it wants to unregister the capability later. (Using the method name means that it doesn't need to remember a separate ID.)

…

On Thu, May 30, 2019 at 3:48 AM Koichi Shiraishi ***@***.***> wrote: @stamblerre <https://github.com/stamblerre> Also, the Registration.ID will use to client/unregisterCapability call. It would be better to use unique ID such as UUID like LSP spec. { "method": "client/registerCapability", "params": { "registrations": [ { "id": "79eee87c-c409-4664-8102-e03263673f6f", "method": "textDocument/willSaveWaitUntil", "registerOptions": { "documentSelector": [ { "language": "javascript" } ] } } ] } } - https://github.com/golang/tools/blob/7be61e1b0e514e106d83fc439d56a79143738603/internal/lsp/general.go#L119 - https://microsoft.github.io/language-server-protocol/specification#client_registerCapability — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#32258?email_source=notifications&email_token=ABJIAI5P6AIFMRKSVCQKLILPX6BFVA5CNFSM4HPW2PJKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODWRUFXA#issuecomment-497238748>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABJIAI7MATL43EY6CE3S76LPX6BFVANCNFSM4HPW2PJA> .

zchee · 2019-05-31T14:46:03Z

@stamblerre @ianthehat @pjweinb
I'll details reply later, but I understand and totally agreed @ianthehat @pjweinb said.

Anyway, I posted issue first means not "should be a pointer", "misuse RegisterCapability call". I think no needs to change to pointer too. I suggests just fix gopls initialized implements.
Because that's specificication like RFC.

stamblerre · 2019-06-05T06:15:52Z

I'm sorry, I misunderstood - I now see that we don't correctly implement didChangeConfiguration.

zchee · 2019-06-05T06:51:24Z

@stamblerre

I'll details reply later

At first, Sorry for not reply to details comment.

But yeah, RegistrationOptions is optional field on LSP spec, but actually it required param for handle RegisterCapability on client side.
So, we should adding any interface{} value to RegistrationOptions instead of change to pointer.

Thanks for reading my comment!

Provide options to disable the now-default fuzzy and/or deep completion modes. Whilst we await golang/go#32258, users will need to set these config options in their .vimrc and then restart Vim. Even then there is a race condition for Vim8 package users that might mean even this doesn't work.

Provide options to disable the now-default fuzzy and/or deep completion modes. Whilst we await golang/go#32258, users will need to set these config options in their .vimrc and then restart Vim. Even then there is a race condition for Vim8 package users that might mean even this doesn't work. Fixes #321

Provide options to disable the now-default fuzzy and/or deep completion modes. Also provide a test to ensure that deep fuzzy is the default. Whilst we await golang/go#32258, users will need to set these config options in their .vimrc and then restart Vim. Even then there is a race condition for Vim8 package users that might mean even this doesn't work. Fixes #321

gopherbot · 2019-11-08T19:18:12Z

Change https://golang.org/cl/206148 mentions this issue: internal/lsp: handle the didChangeConfiguration message

gopherbot added this to the Unreleased milestone May 26, 2019

gopherbot added the gopls Issues related to the Go language server, gopls. label May 26, 2019

julieqiu added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label May 28, 2019

stamblerre changed the title ~~x/tools/internal/lsp: misuse protocol.RegistrationParams~~ x/tools/internal/lsp: protocol RegistrationOptions should be a pointer May 28, 2019

stamblerre changed the title ~~x/tools/internal/lsp: protocol RegistrationOptions should be a pointer~~ x/tools/internal/lsp: protocol.Registration RegistrationOptions field should be a pointer May 28, 2019

stamblerre changed the title ~~x/tools/internal/lsp: protocol.Registration RegistrationOptions field should be a pointer~~ x/tools/internal/lsp: implement didChangeConfiguration Jun 5, 2019

stamblerre mentioned this issue Jun 13, 2019

x/tools/gopls: file not found error #32603

Closed

stamblerre changed the title ~~x/tools/internal/lsp: implement didChangeConfiguration~~ x/tools/gopls: implement didChangeConfiguration Jul 2, 2019

stamblerre added the Suggested Issues that may be good for new contributors looking for work to do. label Jul 10, 2019

stamblerre added help wanted and removed Suggested Issues that may be good for new contributors looking for work to do. labels Aug 8, 2019

stamblerre modified the milestones: Unreleased, gopls v1.0 Aug 26, 2019

myitcv mentioned this issue Sep 7, 2019

cmd/govim: completion options to disable fuzzy or deep matching govim/govim#512

Merged

myitcv mentioned this issue Sep 10, 2019

cmd/govim: add tests for disabling of fuzzy and deep completion govim/govim#520

Open

stamblerre assigned ianthehat Nov 7, 2019

gopherbot closed this as completed in golang/tools@74addff Nov 12, 2019

stamblerre modified the milestones: gopls/v1.0.0, gopls/v0.4.0 Jul 22, 2020

golang locked and limited conversation to collaborators Jul 22, 2021

gopherbot added the FrozenDueToAge label Jul 22, 2021

rsc unassigned ianthehat Jun 23, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

x/tools/gopls: implement didChangeConfiguration #32258

x/tools/gopls: implement didChangeConfiguration #32258

zchee commented May 26, 2019 •

edited

zchee commented May 26, 2019

stamblerre commented May 28, 2019

zchee commented May 30, 2019

ianthehat commented May 30, 2019

pjweinb commented May 31, 2019 via email

zchee commented May 31, 2019 •

edited

stamblerre commented Jun 5, 2019

zchee commented Jun 5, 2019

gopherbot commented Nov 8, 2019

x/tools/gopls: implement didChangeConfiguration #32258

x/tools/gopls: implement didChangeConfiguration #32258

Comments

zchee commented May 26, 2019 • edited

What version of Go are you using (go version)?

Does this issue reproduce with the latest release?

What operating system and processor architecture are you using (go env)?

What did you do?

What did you expect to see?

What did you see instead?

zchee commented May 26, 2019

stamblerre commented May 28, 2019

zchee commented May 30, 2019

ianthehat commented May 30, 2019

pjweinb commented May 31, 2019 via email

zchee commented May 31, 2019 • edited

stamblerre commented Jun 5, 2019

zchee commented Jun 5, 2019

gopherbot commented Nov 8, 2019

zchee commented May 26, 2019 •

edited

What version of Go are you using (`go version`)?

What operating system and processor architecture are you using (`go env`)?

zchee commented May 31, 2019 •

edited