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: support case insensitive file systems #36904

Closed
stamblerre opened this issue Jan 30, 2020 · 9 comments
Closed

x/tools/gopls: support case insensitive file systems #36904

stamblerre opened this issue Jan 30, 2020 · 9 comments
Labels
Milestone

Comments

@stamblerre
Copy link
Contributor

@stamblerre stamblerre commented Jan 30, 2020

gopls doesn't support case-insensitive file systems. VS Code always sends lower-case drive letters, but if a user sets their GOPATH to C://bob/go, go list will return upper-case drive letters. At that point, gopls will think it did not get metadata for the given file, and it will not work correctly.

@stamblerre stamblerre added this to the gopls/v1.0.0 milestone Jan 30, 2020
@stamblerre stamblerre changed the title x/tools/gopls: don't support case insensitive file systems x/tools/gopls: support case insensitive file systems Jan 30, 2020
@gopherbot
Copy link

@gopherbot gopherbot commented Jan 30, 2020

Change https://golang.org/cl/217080 mentions this issue: internal/span: always uppercase the drive letter for Windows

gopherbot pushed a commit to golang/tools that referenced this issue Jan 30, 2020
Drive letters are always case-insensitive, so we should standardize them
by always keeping them uppercase.

Updates golang/go#36904

Change-Id: I8de25b175790b01627f947600c1511edf38c316c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/217080
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
@gopherbot
Copy link

@gopherbot gopherbot commented Jan 30, 2020

Change https://golang.org/cl/217088 mentions this issue: internal/span: always uppercase the drive letter for Windows

gopherbot pushed a commit to golang/tools that referenced this issue Jan 30, 2020
Drive letters are always case-insensitive, so we should standardize them
by always keeping them uppercase.

Updates golang/go#36904

Change-Id: I8de25b175790b01627f947600c1511edf38c316c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/217080
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
(cherry picked from commit 0725381)
Reviewed-on: https://go-review.googlesource.com/c/tools/+/217088
Reviewed-by: Michael Matloob <matloob@golang.org>
@stamblerre
Copy link
Contributor Author

@stamblerre stamblerre commented Mar 12, 2020

@heschik: Do you think there is anything else we can do here to handle case insensitive file systems, or can we close this issue?

@heschik
Copy link
Contributor

@heschik heschik commented Mar 13, 2020

There might be other stuff we can do but in general I would hope that everything would be case-preserving and it wouldn't matter. I say close.

@heschik heschik closed this Mar 13, 2020
@heschik heschik reopened this Mar 19, 2020
@heschik
Copy link
Contributor

@heschik heschik commented Mar 19, 2020

We have a report on Slack of VS Code using the wrong case for initialize and other file URIs, so I guess our work here is not done.

@jameshartig
Copy link
Contributor

@jameshartig jameshartig commented Mar 19, 2020

As I mentioned in Slack, I'm not sure if this is a VSCode or gopls issue. Somehow VSCode opened the folder c:\Users\James\Dropbox\go\src\gerrit.levenlabs.com\subscriber-api but the actual folder is c:\Users\james\Dropbox\go\src\gerrit.levenlabs.com\subscriber-api (notice james not James) and I think that was the root of the issue.

[Trace - 16:23:24.490 PM] Sending request 'initialize - (0)'.
Params: {"processId":15380,"rootPath":"c:\\Users\\James\\Dropbox\\go\\src\\gerrit.levenlabs.com\\subscriber-api","rootUri":"file:///c%3A/Users/James/Dropbox/go/src/gerrit.levenlabs.com/subscriber-api","capabilities":{"workspace":{"applyEdit":true,"workspaceEdit":{"documentChanges":true,"resourceOperations":["create","rename","delete"],"failureHandling":"textOnlyTransactional"},"didChangeConfiguration":{"dynamicRegistration":true},"didChangeWatchedFiles":{"dynamicRegistration":true},"symbol":{"dynamicRegistration":true,"symbolKind":{"valueSet":[1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26]}},"executeCommand":{"dynamicRegistration":true},"configuration":true,"workspaceFolders":true},"textDocument":{"publishDiagnostics":{"relatedInformation":true},"synchronization":{"dynamicRegistration":true,"willSave":true,"willSaveWaitUntil":true,"didSave":true},"completion":{"dynamicRegistration":true,"contextSupport":true,"completionItem":{"snippetSupport":true,"commitCharactersSupport":true,"documentationFormat":["markdown","plaintext"],"deprecatedSupport":true,"preselectSupport":true},"completionItemKind":{"valueSet":[1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25]}},"hover":{"dynamicRegistration":true,"contentFormat":["markdown","plaintext"]},"signatureHelp":{"dynamicRegistration":true,"signatureInformation":{"documentationFormat":["markdown","plaintext"],"parameterInformation":{"labelOffsetSupport":true}}},"definition":{"dynamicRegistration":true,"linkSupport":true},"references":{"dynamicRegistration":true},"documentHighlight":{"dynamicRegistration":true},"documentSymbol":{"dynamicRegistration":true,"symbolKind":{"valueSet":[1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26]},"hierarchicalDocumentSymbolSupport":true},"codeAction":{"dynamicRegistration":true,"codeActionLiteralSupport":{"codeActionKind":{"valueSet":["","quickfix","refactor","refactor.extract","refactor.inline","refactor.rewrite","source","source.organizeImports"]}}},"codeLens":{"dynamicRegistration":true},"formatting":{"dynamicRegistration":true},"rangeFormatting":{"dynamicRegistration":true},"onTypeFormatting":{"dynamicRegistration":true},"rename":{"dynamicRegistration":true,"prepareSupport":true},"documentLink":{"dynamicRegistration":true},"typeDefinition":{"dynamicRegistration":true,"linkSupport":true},"implementation":{"dynamicRegistration":true,"linkSupport":true},"colorProvider":{"dynamicRegistration":true},"foldingRange":{"dynamicRegistration":true,"rangeLimit":5000,"lineFoldingOnly":true},"declaration":{"dynamicRegistration":true,"linkSupport":true}}},"initializationOptions":{},"trace":"off","workspaceFolders":[{"uri":"file:///c%3A/Users/James/Dropbox/go/src/gerrit.levenlabs.com/subscriber-api","name":"subscriber-api"}]}
@stamblerre stamblerre modified the milestones: gopls/v1.0.0, gopls/v0.4.0 Mar 19, 2020
@gopherbot
Copy link

@gopherbot gopherbot commented Mar 26, 2020

Change https://golang.org/cl/225239 mentions this issue: internal/lsp/cache: validate workspace path case

gopherbot pushed a commit to golang/tools that referenced this issue Mar 27, 2020
On case-insensitive file systems, the editor may send us a path that
works but doesn't match the actual file's case. Then when we run go
list, we'll get mismatching paths and everything will break.

We can't reliably fix this problem: tracking what case the editor
expects is too difficult to be worth it. Instead, check the workspace
path and bail if it's mismatched.

Possibly we should also check files on DidOpen or such, but we can start
with this.

Updates golang/go#36904.

Change-Id: I7635c8136bf9400db4143a0f2fde25c39b5abc44
Reviewed-on: https://go-review.googlesource.com/c/tools/+/225239
Reviewed-by: Ian Cottrell <iancottrell@google.com>
@stamblerre
Copy link
Contributor Author

@stamblerre stamblerre commented Apr 1, 2020

@heschik: Is there any more work to do for this issue?

@heschik
Copy link
Contributor

@heschik heschik commented Apr 1, 2020

Check it on file open? But we can wait for the next complaint if any.

@heschik heschik closed this Apr 1, 2020
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.