Skip to content
This repository has been archived by the owner on Feb 22, 2020. It is now read-only.

Use @sourcegraph/lsp-client #28

Open
felixfbecker opened this issue Feb 20, 2019 · 13 comments · May be fixed by #36
Open

Use @sourcegraph/lsp-client #28

felixfbecker opened this issue Feb 20, 2019 · 13 comments · May be fixed by #36
Assignees

Comments

@felixfbecker
Copy link
Contributor

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

@chrismwendt
Copy link
Contributor

Ooh sweet, will do!

@chrismwendt
Copy link
Contributor

chrismwendt commented Mar 5, 2019

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

  • 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.
  • go-langserver expects originalRootUri to be set in the InitParams, not sure how necessary this is, though.

I submitted a PR that fixes these two: sourcegraph/lsp-client#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 #36 (comment)

image

Here's a WIP for sourcegraph-go #36 in case you want to try to repro locally.

@felixfbecker
Copy link
Contributor Author

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
Copy link
Contributor

chrismwendt commented Mar 5, 2019

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. #36 (comment)

Yeah, you can see it in the screenshot:

image

@felixfbecker
Copy link
Contributor Author

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
Copy link
Contributor

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
Copy link
Contributor Author

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
Copy link
Contributor

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
Copy link
Contributor

@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
Copy link
Contributor

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
Copy link
Contributor

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?

ctx.subscriptions.add(
registerWhile({
register: () =>
sourcegraph.languages.registerReferenceProvider([{ pattern: '*.go' }], {
provideReferences: (doc: sourcegraph.TextDocument, pos: sourcegraph.Position) =>
xrefs({
doc,
pos,
sendRequest,
}).pipe(
scan((acc: XRef[], curr: XRef) => [...acc, curr], [] as XRef[]),
map(response => convert.xreferences({ references: response }))
),
}),
settingsPredicate: settings => Boolean(settings['go.showExternalReferences']),
})
)

@chrismwendt
Copy link
Contributor

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

@chrismwendt
Copy link
Contributor

chrismwendt commented May 28, 2019

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

Next steps:

  • Use the @sourcegraph/lsp-client package Use lsp-client #36
  • Send out the blog post
  • Notify impacted customers
  • ... wait a month, then remove the old code

@chrismwendt chrismwendt modified the milestones: 3.3, 3.5 Jun 3, 2019
@chrismwendt chrismwendt modified the milestones: 3.5, 3.6 Jun 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants