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

Bogus workspaceFolders when initializing an LSP server #4436

Closed
artempyanykh opened this issue Oct 23, 2022 · 2 comments · Fixed by #5748
Closed

Bogus workspaceFolders when initializing an LSP server #4436

artempyanykh opened this issue Oct 23, 2022 · 2 comments · Fixed by #5748
Assignees
Labels
A-language-server Area: Language server client C-bug Category: This is a bug

Comments

@artempyanykh
Copy link
Contributor

artempyanykh commented Oct 23, 2022

Summary

See this comment (and related user bug report) for more information.

TL;DR: something recently between 66276ce..ce469ab in Helix such that when helix opens a file which is outside of VSC or any of the roots for the language, the LSP server (Marksman in this example) is initialized with a workspaceFolder equal to cwd.

Workspace folder is used as a project root and a root folder from which an LSP server would hydrate its state which usually means reading a bunch of files in the folder's file tree. When the workspace folder is just cwd, this doesn't make sense.

Of course, I can add some extra logic in Marksman to detect whether it's init inside a project to avoid the file tree scan. But so far, no other editor does what helix does.

Reproduction Steps

See this for repro instructions.

Helix log

No response

Platform

macOS

Terminal Emulator

iTerm2

Helix Version

22.08.1-334-gce469abf

@artempyanykh artempyanykh added the C-bug Category: This is a bug label Oct 23, 2022
@the-mikedavis the-mikedavis added the A-language-server Area: Language server client label Oct 24, 2022
@the-mikedavis
Copy link
Member

Currently if we can't determine a good workspace root we fall back to the CWD and that value gets plugged in to the workspaceFolders value in the initialization params. What would be the preferred behavior in this case? Giving null, [], or defaulting to another location?

@artempyanykh
Copy link
Contributor Author

Both null and [] would work, although null might be a bit more idiomatic as per the LSP doc:

/// ...
/// It can be `null` if the client supports workspace folders but none are configured.
workspaceFolders?: WorkspaceFolder[] | null

The problem with random values in workspaceFolders is that the server can't tell if this is a genuine project or not without doing project-detection logic on its own (that may differ between editors).

Having null for workspaceFolders is fine because when the server gets textDocument/didOpen it'll check the lack of a folder and may enter a single-file mode (if the server supports it) or just do nothing but gracefully :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-language-server Area: Language server client C-bug Category: This is a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants