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.

Sign up for GitHub

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: signatureHelp should return position info for signatures #32468

Open
myitcv opened this issue Jun 6, 2019 · 11 comments
Open

x/tools/gopls: signatureHelp should return position info for signatures #32468

myitcv opened this issue Jun 6, 2019 · 11 comments
Labels
FeatureRequest gopls Issues related to the Go language server, gopls. help wanted Tools This label describes issues relating to any tools in the x/tools repository.
Milestone
gopls/unplanned

Comments

@myitcv
Copy link
Member

myitcv commented Jun 6, 2019

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

$ go version
go version devel +385b2e0cac Fri May 24 21:34:53 2019 +0000 linux/amd64
$ go list -m golang.org/x/tools
golang.org/x/tools v0.0.0-20190521203540-521d6ed310dd

Does this issue reproduce with the latest release?

Yes

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

go env Output
$ go env
GO111MODULE="on"
GOARCH="amd64"
GOBIN="/home/myitcv/gostuff/src/github.com/myitcv/govim/cmd/govim/.bin"
GOCACHE="/home/myitcv/.cache/go-build"
GOENV="/home/myitcv/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/myitcv/gostuff"
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/home/myitcv/gos"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/myitcv/gos/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/myitcv/gostuff/src/github.com/myitcv/govim/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 -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build579955727=/tmp/go-build -gno-record-gcc-switches"

The signature help call from client to server (https://github.com/Microsoft/language-server-protocol/blob/gh-pages/specification.md#signature-help-request-leftwards_arrow_with_hook) returns a list of signatures:

type SignatureHelp struct {
	Signatures []SignatureInformation `json:"signatures"`
	ActiveSignature float64 `json:"activeSignature"`
	ActiveParameter float64 `json:"activeParameter"`
}

type SignatureInformation struct {
	Label string `json:"label"`
	Documentation string `json:"documentation,omitempty"` 
	Parameters []ParameterInformation `json:"parameters,omitempty"`
}

type ParameterInformation struct {
	Label string `json:"label"` 
	Documentation string `json:"documentation,omitempty"` 
}

But what's strange is that the signature information does not include position information, i.e. the start (and end, where it's valid/available) of where the call signature is valid: think the starting ( (to the end )).

This position information is valuable to my mind because it tells the client where to place, for example, a popup window.

cc @stamblerre @ianthehat
@myitcv myitcv added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. gopls Issues related to the Go language server, gopls. labels Jun 6, 2019
@gopherbot gopherbot added this to the Unreleased milestone Jun 6, 2019
@myitcv myitcv mentioned this issue Jun 6, 2019
Open
@stamblerre
Copy link
Contributor

According to the LSP spec, the ParameterInformation struct can include position information instead of a label. If we used that, would it be enough to solve this problem?

@myitcv
Copy link
Member Author

myitcv commented Jun 7, 2019

Oh wow, I totally missed that comment, thanks.

Yes, if this is position information like other places in the spec, should be fine!

@stamblerre
Copy link
Contributor

@pjweinb: Would it be possible for us to make this change in the protocol generator?

@pjweinb
Copy link

pjweinb commented Jun 10, 2019 via email

@pjweinb
Copy link

pjweinb commented Jun 12, 2019

@stamblerre: could you add a [2]float64 field to ParameterInformation in lsp/source/signature_help.go? It does't matter what you name it, but once it exists (and gets filled in), we can change line 57 in signature_help.go to use the new field in place of p.Label.

@stamblerre stamblerre changed the title x/tools/cmd/gopls: signatureHelp should return position info for signatures x/tools/gopls: signatureHelp should return position info for signatures Jul 2, 2019
@myitcv
Copy link
Member Author

myitcv commented Jul 8, 2019

@stamblerre @pjweinb - is there anything I can do to help move this along?

@stamblerre
Copy link
Contributor

I'm sorry, this is a slightly lower priority issue for us right now. I will have to make the changes Peter requested before we can change the generator, but I haven't had a chance to get to this yet.

@stamblerre stamblerre added the Suggested Issues that may be good for new contributors looking for work to do. label Jul 10, 2019
@stamblerre 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 stamblerre modified the milestones: Unreleased, gopls unplanned Dec 4, 2019
@oblitum
Copy link

oblitum commented Jan 25, 2020

For what is worth, I guess it's the same issue, this was fixed on the bingo server long ago saibing/bingo#20.

@oblitum
Copy link

oblitum commented Jan 25, 2020

In specific there was a mention to labelOffsetSupport: saibing/bingo#20 (comment).

@oblitum
Copy link

oblitum commented Jan 25, 2020

And the advent of the gopls server at the time caused a regression for bingo: saibing/bingo#135 (comment).

@stamblerre stamblerre added FeatureRequest and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Jan 25, 2020
@stamblerre
Copy link
Contributor

Thanks for reporting this. It's a slightly different issue because it's a bug in signature help, whereas this issue is to track making use of the labelOffsetSupport capability, as you said. I filed #36766 to track the bug you mentioned.

@stamblerre stamblerre modified the milestones: gopls unplanned, gopls/v1.0.0 Jan 29, 2020
@stamblerre stamblerre modified the milestones: gopls/v1.0.0, gopls/v0.4.0 Feb 2, 2020
@stamblerre stamblerre modified the milestones: gopls/v0.4.0, gopls/v1.0.0 Feb 18, 2020
@stamblerre stamblerre added the Tools This label describes issues relating to any tools in the x/tools repository. label Mar 12, 2020
@stamblerre stamblerre modified the milestones: gopls/v1.0.0, gopls/v0.5.0 May 21, 2020
@stamblerre stamblerre removed this from the gopls/v0.5.0 milestone Jun 20, 2020
@stamblerre stamblerre added this to the gopls/unplanned milestone Oct 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
FeatureRequest gopls Issues related to the Go language server, gopls. help wanted 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
@oblitum @myitcv @pjweinb @stamblerre @gopherbot