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: consider avoiding the workspace/configuration request #57329

Open
findleyr opened this issue Dec 15, 2022 · 1 comment
Open

x/tools/gopls: consider avoiding the workspace/configuration request #57329

findleyr opened this issue Dec 15, 2022 · 1 comment
Labels
gopls Issues related to the Go language server, gopls. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone

Comments

@findleyr
Copy link
Contributor

Right now, gopls uses the workspace/configuration request to resolve configuration of workspace folders.

As described in #54559 (comment), it is odd to manage configuration server-side, when all other session state is managed client-side. Furthermore, as described in microsoft/language-server-protocol#972, the workspace/configuration request poses problems for clients, as they need to specialize to various servers.

IIUC, the only reason gopls makes workspace/configuration requests is to support different settings for different workspace folders. In particular, this allows for different sets of build tags per folder, which is a workaround for lacking build tag support in gopls (#29202). I'm not sure whether anyone is actually using this workaround.

We should investigate if we can change gopls' configuration model to simply rely on initialization options and didChangeConfiguration payloads.

@findleyr findleyr added this to the gopls/later milestone Dec 15, 2022
@gopherbot gopherbot added Tools This label describes issues relating to any tools in the x/tools repository. gopls Issues related to the Go language server, gopls. labels Dec 15, 2022
@findleyr findleyr added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Dec 15, 2022
@hyangah
Copy link
Contributor

hyangah commented Dec 16, 2022

IIUC, the only reason gopls makes workspace/configuration requests is to support different settings for different workspace folders. In particular, this allows for different sets of build tags per folder, which is a workaround for lacking build tag support in gopls (#29202). I'm not sure whether anyone is actually using this workaround.

In VS Code, per-folder setting is an essential part of handling multi-root workspace environment (https://code.visualstudio.com/docs/editor/multi-root-workspaces#_settings). I am afraid dropping per-root folder configuration without an alternative solution will result in more confusion for most VS Code users.

Multi-root workspace feature allows users to create a workspace the users interact with day-to-day in one window. For example, a user can have her small size personal project that depends on a k8s module and the k8s project in one VS Code workspace. In this setting a user may want to enable more expensive analyzers for her own project, but that doesn't necessarily mean she also wants to enable them for all projects in the VS Code workspace. Similarly, a user may want to toggle the new vulncheck scanning feature in one folder, but doesn't want to enable it in all projects in the workspace. Without workspace/configuration, implementing this behavior will be challenging (unless we choose to start a separate gopls instance per each registered folder, which I don't think is great).

My interpretation of microsoft/language-server-protocol#972 is that it is about the protocol design, not about dropping the per-folder configuration feature that relies on LSP workspace/configuration.

As mentioned in #54559 (comment) this functionality is controlled by the workspace.configuration capability, and editors that cannot handle this properly shouldn't advertise the capability.

Re: #54559 (comment)

The problem is that the documentSymbol result in (3) depends on the configuration returned in (2); we don't produce semantic tokens without packages, and we don't load packages without a Go environment, and we don't have a Go environment without configuration. Therefore, if the configuration request in (2) is blocked, gopls has the choice of either serving a degraded result in (3), or timing out.

I think it's reasonable for language servers to block those requests until the configuration is ready, unless the client indicates the requests are server cancellable (LSP 3.17 severCancelSupport - from microsoft/vscode#161627 (comment) I guess VS supports that only for semantic tokens). Clients will need to cancel the request after some time or wait patiently depending on their need.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gopls Issues related to the Go language server, gopls. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Development

No branches or pull requests

3 participants