OmniSharp / OmniSharp/csharp-language-server-protocol

Issue with GetConfiguration when `workspace.configuration` capability is declared with the `workspace.didChangeConfiguration` capability

Open
#1,101 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
C#
Stars
638
Forks
109
Avg merge
1m
Merged PRs (30d)
2

Description

Hi there,

My language server recently started getting used with a language client (neovim lsp) that provides the following intialize extract for the client workspace capabilities:

"workspace": {
   "applyEdit": true,
   "configuration": true,
   "didChangeWatchedFiles": {
      "dynamicRegistration": true,
      "relativePatternSupport": true
   },
   "inlayHint": {"refreshSupport": true},
   "semanticTokens": {"refreshSupport": true},
   "symbol": {
      "dynamicRegistration": false,
      "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 ]
      }
   },
   "workspaceEdit": {
      "resourceOperations": ["rename", "create", "delete"]
   },
   "workspaceFolders": true
}

As you can see, workspace.configuration == true, but there is no workspace.didChangeConfiguration.

In the current OmniSharp implementation, the API to trigger a workspace/configuration request from server to client, seems to be in two places in the DidChangeConfigurationProvider class:

The former GetConfiguration includes a check for _capability == null and will not send the workspace/configuration request if it is null. Unfortunately, this capability is only true if the workspace.didChangeConfiguration client capability is declared. So in my case, no request is issued, even if the language client does support workspace.configuration.

The latter GetScopedConfiguration does not include a capability check. So in my case, I've been able to work around it by switching to scoped configuration. However, this is also not technically correct, as both methods should do a capability check - just for the workspace.configuration capability, not the workspace.didChangeconfiguration capability.

I'd be happy to prepare a PR, but some guidance would be helpful, as there seems to be a lot of autogenerated code in the handling of the capabilities, and the handling of both configuration changes, and configuration retrieval in the same class DidChangeConfigurationProvider might be confusing things.

If someone can provide a high level description of the appropriate way to address this, I'd be happy to execute it.

Thanks!

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in Server/Configuration/DidChangeConfigurationProvider.cs, examining GetConfiguration and GetScopedConfiguration and how client capabilities are represented. Trace the checks for workspace.configuration and workspace.didChangeConfiguration; done means configuration retrieval checks the supported capability without incorrectly requiring the change-notification capability, consistently in both methods.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp
Domain
api
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.