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: client Configuration made with identical items? #33347

Closed
myitcv opened this issue Jul 29, 2019 · 7 comments
Closed

x/tools/gopls: client Configuration made with identical items? #33347

myitcv opened this issue Jul 29, 2019 · 7 comments

Comments

@myitcv
Copy link
Member

@myitcv myitcv commented Jul 29, 2019

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

$ go version
go version go1.13beta1 linux/amd64
$ go list -m golang.org/x/tools
golang.org/x/tools v0.0.0-20190729092621-ff9f1409240a
$ go list -m golang.org/x/tools/gopls
golang.org/x/tools/gopls v0.1.4-0.20190729092621-ff9f1409240a

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=""
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"
GOPRIVATE=""
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-build626969461=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Following CL 187819, gopls appears to be ignoring the value returned by the client's implementation of Configuration. That said, this could, once again, be an issue in govim's implementation 😄

Raising here for discussion.

In the test case in question, govim returns:

{"hoverKind":"NoDocumentation","noDocsOnHover":true}

However, a call to textDocument/hover still returns the synopsis documentation.

What did you expect to see?

The client-returned Configuration value to be observed.

What did you see instead?

As above.


cc @stamblerre @ianthehat

@gopherbot gopherbot added this to the Unreleased milestone Jul 29, 2019
@myitcv myitcv changed the title x/tools/cmd/gopls: CL 187819 appears to cause client returned Configuration to be ignored x/tools/gopls: CL 187819 appears to cause client returned Configuration to be ignored Jul 29, 2019
@stamblerre

This comment has been minimized.

Copy link
Contributor

@stamblerre stamblerre commented Aug 5, 2019

Can you share what the RPC trace looks like for this in your gopls logs?

@myitcv

This comment has been minimized.

Copy link
Member Author

@myitcv myitcv commented Aug 12, 2019

Actually the issue appears to be that the Configuration call is made with two identical items and govim currently doesn't know what to do in that case:

Configuration: &protocol.ConfigurationParams{
    Items: {
        {ScopeURI:"file:///home/myitcv/gostuff/src/github.com/myitcv/playground", Section:"gopls"},
        {ScopeURI:"file:///home/myitcv/gostuff/src/github.com/myitcv/playground", Section:"playground"},
    },
}

Is there a difference between these two?

@myitcv myitcv changed the title x/tools/gopls: CL 187819 appears to cause client returned Configuration to be ignored x/tools/gopls: client Configuration made with identical items? Aug 12, 2019
@stamblerre

This comment has been minimized.

Copy link
Contributor

@stamblerre stamblerre commented Aug 12, 2019

I believe that this was to allow users to set per-folder configurations in their settings, but @ianthehat can probably clarify this further.

@ianthehat

This comment has been minimized.

Copy link

@ianthehat ianthehat commented Aug 12, 2019

It will ask for the global "gopls" configuration as well as one per workspace folder (in this case "playground") using the name the workspace folder was given.
It does it this way because the same folder can be opened more than once, with different configurations so long as the name is different.
This would allow you for instance to open the same folder twice with differing build tags.

@myitcv

This comment has been minimized.

Copy link
Member Author

@myitcv myitcv commented Aug 12, 2019

Thanks. A few follow up questions (which we can move elsewhere if you'd prefer):

  • What is the relationship between the "gopls" configuration and the "playground" configuration?
  • In this case I don't believe I did anything explicit with regards to the workspace folder. Where does the name "playground" come from?
  • Would the absolute path to the directory not be better than simply "playground" (which I'm assuming is the base of the directory containing the go.mod)
@ianthehat

This comment has been minimized.

Copy link

@ianthehat ianthehat commented Aug 12, 2019

The "gopls" configuration is global, and applied first, the "playground" configuration is workspace folder specific and applied after, it can thus override things from the global configuration if needed.

Workspace folders always have a name (https://microsoft.github.io/language-server-protocol/specification#workspace_workspaceFolders). If you are not using workspace folders and just specifying a root uri, then we make a workspace folder for you which has a name set from the basename of the root uri.

The path is not unique, only the name is. For instance, you could open {uri:"file:///home/myitcv/gostuff/src/github.com/myitcv/playground",name:"playground_windows"} and {uri:"file:///home/myitcv/gostuff/src/github.com/myitcv/playground",name:"playground_osx"} at the same time, and respond with appropriate configurations such that they have the right GOOS and represent that folder for the two OSs.

@myitcv

This comment has been minimized.

Copy link
Member Author

@myitcv myitcv commented Aug 13, 2019

Thanks very much for the detailed explanation. There's clearly no issue here on the gopls side, so I'll close this.

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
4 participants
You can’t perform that action at this time.