Skip to content
This repository has been archived by the owner on Oct 12, 2022. It is now read-only.

Support git:// in the rootUri #373

Merged
merged 6 commits into from
May 28, 2019
Merged

Support git:// in the rootUri #373

merged 6 commits into from
May 28, 2019

Conversation

chrismwendt
Copy link
Contributor

@chrismwendt chrismwendt commented May 14, 2019

This change only affects the buildserver, which is only used by Sourcegraph. There is no change to editor users.

Motivation

I'm working on integrating lsp-client into the Go extension, which has surfaced a non-spec-compliant characteristic of go-langserver:

  • originalRootUri must be specified in the InitializeParams

Instead of adding workarounds in lsp-client, I'm adding support in go-langserver for putting the value of originalRootUri in the rootUri field and omitting originalRootUri.

Backwards compatibility

During a migration period in which customers bump their go-langserver versions, there will be some customers running the old version and some running the new version.

My current plan is to only merge lsp-client into the Go extension when customers have bumped their versions. This means that in the meantime, go-langserver must remain compatible with the current Go extension (i.e. it must still support originalRootUri). That's why you'll see the flag clientUsesFileSchemeWithinWorkspace and some if-statements in this PR.

Details

In the rootUri in the InitializeParams of the initialize request, clients can send either:

  • (not new in this PR) A file:// URI, which indicates that:
    • Same-workspace file paths will also be file:// URIs
    • Out-of-workspace file paths will be git:// URIs
    • originalRootUri is present
  • (new in this PR) A git:// URI, which indicates that:
    • Both same-workspace and out-of-workspace file paths will be non-file:// URIs
    • originalRootUri is absent and rootUri contains the original root URI

This subsumes the other approach #369

buildserver/build_server.go Outdated Show resolved Hide resolved
buildserver/proxy_test.go Outdated Show resolved Hide resolved
buildserver/build_server.go Outdated Show resolved Hide resolved
@chrismwendt
Copy link
Contributor Author

Thanks for the review 🙇

@chrismwendt chrismwendt merged commit 02f4198 into master May 28, 2019
@chrismwendt chrismwendt deleted the git-root-uri branch May 28, 2019 18:21
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 this pull request may close these issues.

3 participants