search

sourcegraph / sourcegraph-go

Provides code intelligence for Go
6 stars 5 forks source link

Use @sourcegraph/lsp-client #28

Open felixfbecker opened 5 years ago

felixfbecker commented 5 years ago

@sourcegraph/lsp-client is ready to use, please adopt it and give feedback :)

chrismwendt commented 5 years ago

Ooh sweet, will do!

chrismwendt commented 5 years ago

Ok, so I tried it out and it didn't work out of the box, but the issues appear fixable:

I submitted a PR that fixes these two: https://github.com/sourcegraph/lsp-client/pull/11

However, even after that, it looks like lsp-client just go stuck after the initialize request and didn't send any hover requests: Edit I had the wrong document selector https://github.com/sourcegraph/sourcegraph-go/pull/36#discussion_r262581023

image

Here's a WIP for sourcegraph-go https://github.com/sourcegraph/sourcegraph-go/pull/36 in case you want to try to repro locally.

felixfbecker commented 5 years ago

Thanks for trying it!

  • I need a way to set initializationOptions (to include zipURL, which is based on the rootUri). I think this has to be different from the rootUri because the go-langserver uses the rootUri to determine the import path or something.

Why can't it figure out the import path from rootUri?

  • go-langserver expects originalRootUri to be set in the InitParams, not sure how necessary this is, though.

What is it used for / what is it set to?

However, even after that, it looks like lsp-client just go stuck after the initialize request and didn't send any hover requests:

image

Does the Go langserver return the hover capability?

chrismwendt commented 5 years ago

Why can't it figure out the import path from rootUri?

You mean go-langserver could be taught how to strip the noise like /-/raw from rootUri to get the github.com/org/repo part?

I was curious what go-langserver did with the rootUri, and I found that it mounts the files in the VFS at a path like $GOPATH/src/<rootUri> https://github.com/sourcegraph/go-langserver/blob/f9fdcbb3871320e0655530b707a3d276d561311a/langserver/handler_common.go#L35

What is it (originalRootUri) used for / what is it set to?

It's pretty much the same as rootUri, needs more investigation to determine whether or not we could drop it and use rootUri.

Does the Go langserver return the hover capability?

Edit I had the wrong document selector. https://github.com/sourcegraph/sourcegraph-go/pull/36#discussion_r262581023

Yeah, you can see it in the screenshot:

image

felixfbecker commented 5 years ago

You mean go-langserver could be taught how to strip the noise like /-/raw from rootUri to get the github.com/org/repo part?

I mean, it already needs to do something right now since the rootUri can't be set to github.com/org/repo

chrismwendt commented 5 years ago

Oh, that's what originalRootUri is for. Maybe we can feed two birds with one seed by moving originalRootUri into initializationOptions and letting consumers of lsp-client set custom values in initializationOptions?

felixfbecker commented 5 years ago

I in any way possible I would like to avoid supporting things like that because it's against the indented use of initializationOptions and won't be compatible with multi-root support.

How does Go know from looking at a file which import path it has?

chrismwendt commented 5 years ago

Then I need to determine whether or not go-langserver can work without knowing where to mount the repo in the VFS, which deprioritizes this in my TODO list https://github.com/sourcegraph/sourcegraph/issues/2452

chrismwendt commented 5 years ago

@felixfbecker I'd be happy to implement custom initializationOptions in lsp-client 😃 I'll be responsible for any use of it in the Go extension.

chrismwendt commented 5 years ago

originalRootUri must be passed at the top level because some users are running old/current versions of go-langserver which expect it to be there.

chrismwendt commented 5 years ago

Ok, I got lsp-client (mostly) working with these changes:

Xrefs don't work yet. Any idea how lsp-client can support custom references providers such as this?

https://github.com/sourcegraph/sourcegraph-go/blob/feb0a79918ede42f3e2c1a7678d22d10abc28578/src/lang-go.ts#L654-L670

chrismwendt commented 5 years ago

Oh, I think the Go extension can use the LSPClient that's returned by register.

chrismwendt commented 5 years ago

sourcegraph-go:insiders@sha256:9b2eb14e6acdd41c3e7fe312b79a72200efa4cac096eb13e2ecd86ee67821a62 was pushed 10 minutes ago with support for more lsp-compliant initialization options.

Next steps: