Use a config variable to store configuration changes

[erikjuhani]

Please check if the PR fulfills these requirements**

• The commit message follows our guidelines

Where is commit message guidelines?

• Tests for the changes have been added (for bug fixes / features)

Adding resilience so I don't think new test cases need to be formulated.

• Docs have been added / updated (for bug fixes / features)

Context

When using for example the neovim language server client, the contextive language server does not acquire the proper settings sent by the client in the OnStarted handler. However, the settings are passed correctly using the OnDidChangeConfiguration handler.

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)

This change introduces a scoped variable in the server start function to hold a mutable field for the Contextive path configuration. The configuration variable is initialized with the default path for Contextive definitions introduced in commit 23f9049. If the language configuration does not have a config value, it will use the value from the configuration variable instead. This works the same way as it did previously, but the source is now a parameter of the function.

What is the current behavior? (You can also link to an open issue here)

Currently the configuration initialization does not work correctly with nvim (v0.9.2) language client server. I've spent multiple hours trying to understand the cause for this, but alas I'm empty handed here. There must be something that is not right, maybe it could be an OmniSharp issue too.

Other information:

This work is related to #33

[erikjuhani]

I have a setup like this for the contextive language server with custom settings for the contextive path:

lspconfig.contextive.setup {
  settings = {
    contextive = {
      path = vim.loop.cwd() .. "/.contextive.yml"
    }
  },
  on_init = function(client)
    if next(client.config.settings) then
      client.notify('workspace/configuration', { settings = client.config.settings })
      client.notify('workspace/didChangeConfiguration', { settings = client.config.settings })
    end
  end
}

The additional client.notify is just to make sure that the notification is sent (This happens automatically on neovim 0.8 onwards). The above configuration does not work with the current contextive language server, but with the changes in the PR this is possible. I'm not sure why this happens and I'm also eager to hear your thoughts on this implementation.

[chrissimon-au]

It's quite puzzling to me as to why this would work:

In the getConfig function, this line triggers the LSP's workspace/configuration request from server to client. The only reason your change could make any difference is if that request was not being handled by the neovim lsp client. However the lsp client specifically says:

settings: Map with language server specific settings. These are returned to the language server if requested via workspace/configuration. Keys are case-sensitive.

One possibility is that the Omnisharp library will not even make the request if the client has not advertised the configuration capability. I would be interested to see what capabilities the client is advertising.

So something that would help is if you can find a way to enable protocol-level logging of the LSP messages. I've found that a trace of the LSP messages sent back and forth usually sheds a lot of light on what's going on with these things - including the initialize method which contains the advertised capabilities.

Also odd is that your setting includes:

client.notify('workspace/configuration', { settings = client.config.settings })

But workspace/configuration is not a notification - it's a request sent from server to client. So the client should be registering a handler for that request, not sending it as a notification. I suspect that the server is ignoring or erroring on receiving that notification. But is that the 'extra' notify you were referring to?

Further Thoughts

Interestingly, the way the config is making it through with this PR is that you are capturing the value provided in the didChangeConfiguration notification, storing it in a mutable, and then providing it if the getConfig method wasn't able to obtain a value from the workspace/configuration requestion.

However if you think about it, when we get didChangeNotification, we are triggering a reload of the definitions file. We currently pass in a delegate that calls back to the getConfig method - but if we have the actual value, we could just pass in the value and eliminate the delegate (and mutable) altogether.

I think the reason I didn't do that initially is that an older version of the Omnisharp library didn't make the config settings available properly in the notification, so I had no choice but to use the didChangeConfiguration notification only to trigger a reload of the definitions (not to capture the value), and in the reload get the value by using the workspace/configuration request.

I'll do some testing because if neovim and vscode and omnisharp now all properly work together to provide the value in the didchangeconfiguration notification, then the whole structure can be much simpler - we can just pass the new config value directly to the definitions reloader!

This is a pretty major change, but if it is possible, would be worth it - I should have a chance to look into it in the next day or so.

[erikjuhani]

It's quite puzzling to me as to why this would work:

In the getConfig function, this line triggers the LSP's workspace/configuration request from server to client. The only reason your change could make any difference is if that request was not being handled by the neovim lsp client. However the lsp client specifically says:

settings: Map with language server specific settings. These are returned to the language server if requested via workspace/configuration. Keys are case-sensitive.

One possibility is that the Omnisharp library will not even make the request if the client has not advertised the configuration capability. I would be interested to see what capabilities the client is advertising.

I can print out the capabilities and configuration from the neovim directly.

This is the workspace configuration:

workspace = {
  applyEdit = true,
  configuration = true,
  didChangeWatchedFiles = <table 19>,
  semanticTokens = <table 20>,
  symbol = <table 21>,
  workspaceEdit = <table 22>,
  workspaceFolders = true
}

and these are the capabilities:

capabilities = {
  textDocument = {
    callHierarchy = <table 1>,
    codeAction = <table 2>,
    completion = {
      completionItem = {
        commitCharactersSupport = true,
        deprecatedSupport = true,
        documentationFormat = <table 3>,
        insertReplaceSupport = true,
        insertTextModeSupport = <26>{
          valueSet = { 1, 2 }
        },
        labelDetailsSupport = true,
        preselectSupport = true,
        resolveSupport = <27>{
          properties = { "documentation", "detail", "additionalTextEdits", "sortText", "filterText", "insertText", "textEdit", "insertTextFormat", "insertTextMode" }
        },
        snippetSupport = true,
        tagSupport = <28>{
          valueSet = { 1 }
        }
      },
      completionItemKind = <table 4>,
      completionList = <29>{
        itemDefaults = { "commitCharacters", "editRange", "insertTextFormat", "insertTextMode", "data" }
      },
      contextSupport = true,
      dynamicRegistration = false,
      insertTextMode = 1
    },
    declaration = <table 5>,
    definition = <table 6>,
    documentHighlight = <table 7>,
    documentSymbol = <table 8>,
    hover = <table 9>,
    implementation = <table 10>,
    publishDiagnostics = <table 11>,
    references = <table 12>,
    rename = <table 13>,
    semanticTokens = <table 14>,
    signatureHelp = <table 15>,
    synchronization = <table 16>,
    typeDefinition = <table 17>
  },

Maybe you can see something out of place in that data?

So something that would help is if you can find a way to enable protocol-level logging of the LSP messages. I've found that a trace of the LSP messages sent back and forth usually sheds a lot of light on what's going on with these things - including the initialize method which contains the advertised capabilities.

I can try to enable the debugging for the lsp protocol and paste the logs here.

Also odd is that your setting includes:

client.notify('workspace/configuration', { settings = client.config.settings })

But workspace/configuration is not a notification - it's a request sent from server to client. So the client should be registering a handler for that request, not sending it as a notification. I suspect that the server is ignoring or erroring on receiving that notification. But is that the 'extra' notify you were referring to?

Yes that is the 'extra' I mentioned. I am quite puzzled myself too as why this isn't working. Everything seems to be how it should. I have even ventured to omnisharp source code to understand it better. There is a mention in the microsoft language server extension guide however that the language server should support a situation where workspace is not supported. It is hidden in the example code comments:

// The global settings, used when the `workspace/configuration` request is not supported by the client.
// Please note that this is not the case when using this server with the client provided in this example
// but could happen with other clients.
const defaultSettings: ExampleSettings = { maxNumberOfProblems: 1000 };
let globalSettings: ExampleSettings = defaultSettings;

// Cache the settings of all open documents
let documentSettings: Map<string, Thenable<ExampleSettings>> = new Map();

connection.onDidChangeConfiguration(change => {
  if (hasConfigurationCapability) {
    // Reset all cached document settings
    documentSettings.clear();
  } else {
    globalSettings = <ExampleSettings>(
      (change.settings.languageServerExample || defaultSettings)
    );
  }

  // Revalidate all open text documents
  documents.all().forEach(validateTextDocument);
});

Further Thoughts

Interestingly, the way the config is making it through with this PR is that you are capturing the value provided in the didChangeConfiguration notification, storing it in a mutable, and then providing it if the getConfig method wasn't able to obtain a value from the workspace/configuration requestion.

However if you think about it, when we get didChangeNotification, we are triggering a reload of the definitions file. We currently pass in a delegate that calls back to the getConfig method - but if we have the actual value, we could just pass in the value and eliminate the delegate (and mutable) altogether.

I think the reason I didn't do that initially is that an older version of the Omnisharp library didn't make the config settings available properly in the notification, so I had no choice but to use the didChangeConfiguration notification only to trigger a reload of the definitions (not to capture the value), and in the reload get the value by using the workspace/configuration request.

I'll do some testing because if neovim and vscode and omnisharp now all properly work together to provide the value in the didchangeconfiguration notification, then the whole structure can be much simpler - we can just pass the new config value directly to the definitions reloader!

This is a pretty major change, but if it is possible, would be worth it - I should have a chance to look into it in the next day or so.

Sounds good to me! Yeah initially I was thinking that probably the right way to change this is actually a bigger overhaul on how the configuration is passed forward. Anyways it feels dirty to have a mutable field. 😅

[erikjuhani]

Here is the LSP debug log. There is a custom path, which is provided with the settings from neovim client.

[INFO][2023-10-22 20:29:13] .../lua/vim/lsp.lua:1875	"exit_handler"	{ {
    _on_attach = <function 1>,
    attached_buffers = { true },
    cancel_request = <function 2>,
    commands = {},
    config = {
      _on_attach = <function 3>,
      autostart = true,
      capabilities = {
        textDocument = {
          callHierarchy = {
            dynamicRegistration = false
          },
          codeAction = {
            codeActionLiteralSupport = {
              codeActionKind = {
                valueSet = { "", "quickfix", "refactor", "refactor.extract", "refactor.inline", "refactor.rewrite", "source", "source.organizeImports" }
              }
            },
            dataSupport = true,
            dynamicRegistration = false,
            isPreferredSupport = true,
            resolveSupport = {
              properties = { "edit" }
            }
          },
          completion = {
            completionItem = {
              commitCharactersSupport = true,
              deprecatedSupport = true,
              documentationFormat = { "markdown", "plaintext" },
              insertReplaceSupport = true,
              insertTextModeSupport = {
                valueSet = { 1, 2 }
              },
              labelDetailsSupport = true,
              preselectSupport = true,
              resolveSupport = {
                properties = { "documentation", "detail", "additionalTextEdits", "sortText", "filterText", "insertText", "textEdit", "insertTextFormat", "insertTextMode" }
              },
              snippetSupport = true,
              tagSupport = {
                valueSet = { 1 }
              }
            },
            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 }
            },
            completionList = {
              itemDefaults = { "commitCharacters", "editRange", "insertTextFormat", "insertTextMode", "data" }
            },
            contextSupport = true,
            dynamicRegistration = false,
            insertTextMode = 1
          },
          declaration = {
            linkSupport = true
          },
          definition = {
            linkSupport = true
          },
          documentHighlight = {
            dynamicRegistration = false
          },
          documentSymbol = {
            dynamicRegistration = false,
            hierarchicalDocumentSymbolSupport = 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 }
            }
          },
          hover = {
            contentFormat = { "markdown", "plaintext" },
            dynamicRegistration = false
          },
          implementation = {
            linkSupport = true
          },
          publishDiagnostics = {
            relatedInformation = true,
            tagSupport = {
              valueSet = { 1, 2 }
            }
          },
          references = {
            dynamicRegistration = false
          },
          rename = {
            dynamicRegistration = false,
            prepareSupport = true
          },
          semanticTokens = {
            augmentsSyntaxTokens = true,
            dynamicRegistration = false,
            formats = { "relative" },
            multilineTokenSupport = false,
            overlappingTokenSupport = true,
            requests = {
              full = {
                delta = true
              },
              range = false
            },
            serverCancelSupport = false,
            tokenModifiers = { "declaration", "definition", "readonly", "static", "deprecated", "abstract", "async", "modification", "documentation", "defaultLibrary" },
            tokenTypes = { "namespace", "type", "class", "enum", "interface", "struct", "typeParameter", "parameter", "variable", "property", "enumMember", "event", "function", "method", "macro", "keyword", "modifier", "comment", "string", "number", "regexp", "operator", "decorator" }
          },
          signatureHelp = {
            dynamicRegistration = false,
            signatureInformation = {
              activeParameterSupport = true,
              documentationFormat = { "markdown", "plaintext" },
              parameterInformation = {
                labelOffsetSupport = true
              }
            }
          },
          synchronization = {
            didSave = true,
            dynamicRegistration = false,
            willSave = true,
            willSaveWaitUntil = true
          },
          typeDefinition = {
            linkSupport = true
          }
        },
        window = {
          showDocument = {
            support = true
          },
          showMessage = {
            messageActionItem = {
              additionalPropertiesSupport = false
            }
          },
          workDoneProgress = true
        },
        workspace = {
          applyEdit = true,
          configuration = true,
          didChangeWatchedFiles = {
            dynamicRegistration = false,
            relativePatternSupport = true
          },
          semanticTokens = {
            refreshSupport = true
          },
          symbol = {
            dynamicRegistration = false,
            hierarchicalWorkspaceSymbolSupport = 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 }
            }
          },
          workspaceEdit = {
            resourceOperations = { "rename", "create", "delete" }
          },
          workspaceFolders = true
        }
      },
      cmd = { "/Users/erik/bin/contextivels" },
      cmd_cwd = "/Users/erik/git/contextive-demo-go-common",
      flags = {},
      get_language_id = <function 4>,
      handlers = <1>{},
      init_options = {
        AutomaticWorkspaceInit = true
      },
      log_level = 2,
      message_level = 2,
      name = "contextive",
      on_attach = <function 5>,
      on_exit = <function 6>,
      on_init = <function 7>,
      root_dir = "/Users/erik/git/contextive-demo-go-common",
      settings = {
        contextive = {
          path = "/Users/erik/git/contextive-demo-go-common/.contextive.yml"
        }
      },
      workspace_folders = <2>{ {
          name = "/Users/erik/git/contextive-demo-go-common",
          uri = "file:///Users/erik/git/contextive-demo-go-common"
        } },
      <metatable> = <3>{
        __tostring = <function 8>
      }
    },
    handlers = <table 1>,
    id = 1,
    initialized = true,
    is_stopped = <function 9>,
    messages = {
      messages = {},
      name = "contextive",
      progress = {},
      status = {}
    },
    name = "contextive",
    notify = <function 10>,
    offset_encoding = "utf-16",
    request = <function 11>,
    request_sync = <function 12>,
    requests = {},
    rpc = {
      is_closing = <function 13>,
      notify = <function 14>,
      request = <function 15>,
      terminate = <function 16>
    },
    server_capabilities = {
      completionProvider = vim.empty_dict(),
      experimental = vim.empty_dict(),
      hoverProvider = vim.empty_dict(),
      textDocumentSync = {
        change = 1,
        openClose = true,
        save = {
          includeText = true
        }
      },
      workspace = {
        fileOperations = vim.empty_dict(),
        workspaceFolders = {
          changeNotifications = true,
          supported = true
        }
      }
    },
    stop = <function 17>,
    supports_method = <function 18>,
    workspace_did_change_configuration = <function 19>,
    workspace_folders = <table 2>
  } }
[DEBUG][2023-10-22 20:29:13] .../vim/lsp/rpc.lua:284	"rpc.send"	{
  id = 2,
  jsonrpc = "2.0",
  method = "shutdown"
}
[START][2023-10-22 20:29:14] LSP logging initiated
[INFO][2023-10-22 20:29:14] .../vim/lsp/rpc.lua:662	"Starting RPC client"	{
  args = {},
  cmd = "/Users/erik/bin/contextivels",
  extra = {
    cwd = "/Users/erik/git/contextive-demo-go-common"
  }
}
[DEBUG][2023-10-22 20:29:14] .../vim/lsp/rpc.lua:284	"rpc.send"	{
  id = 1,
  jsonrpc = "2.0",
  method = "initialize",
  params = {
    capabilities = {
      textDocument = {
        callHierarchy = {
          dynamicRegistration = false
        },
        codeAction = {
          codeActionLiteralSupport = {
            codeActionKind = {
              valueSet = { "", "quickfix", "refactor", "refactor.extract", "refactor.inline", "refactor.rewrite", "source", "source.organizeImports" }
            }
          },
          dataSupport = true,
          dynamicRegistration = false,
          isPreferredSupport = true,
          resolveSupport = {
            properties = { "edit" }
          }
        },
        completion = {
          completionItem = {
            commitCharactersSupport = true,
            deprecatedSupport = true,
            documentationFormat = { "markdown", "plaintext" },
            insertReplaceSupport = true,
            insertTextModeSupport = {
              valueSet = { 1, 2 }
            },
            labelDetailsSupport = true,
            preselectSupport = true,
            resolveSupport = {
              properties = { "documentation", "detail", "additionalTextEdits", "sortText", "filterText", "insertText", "textEdit", "insertTextFormat", "insertTextMode" }
            },
            snippetSupport = true,
            tagSupport = {
              valueSet = { 1 }
            }
          },
          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 }
          },
          completionList = {
            itemDefaults = { "commitCharacters", "editRange", "insertTextFormat", "insertTextMode", "data" }
          },
          contextSupport = true,
          dynamicRegistration = false,
          insertTextMode = 1
        },
        declaration = {
          linkSupport = true
        },
        definition = {
          linkSupport = true
        },
        documentHighlight = {
          dynamicRegistration = false
        },
        documentSymbol = {
          dynamicRegistration = false,
          hierarchicalDocumentSymbolSupport = 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 }
          }
        },
        hover = {
          contentFormat = { "markdown", "plaintext" },
          dynamicRegistration = false
        },
        implementation = {
          linkSupport = true
        },
        publishDiagnostics = {
          relatedInformation = true,
          tagSupport = {
            valueSet = { 1, 2 }
          }
        },
        references = {
          dynamicRegistration = false
        },
        rename = {
          dynamicRegistration = false,
          prepareSupport = true
        },
        semanticTokens = {
          augmentsSyntaxTokens = true,
          dynamicRegistration = false,
          formats = { "relative" },
          multilineTokenSupport = false,
          overlappingTokenSupport = true,
          requests = {
            full = {
              delta = true
            },
            range = false
          },
          serverCancelSupport = false,
          tokenModifiers = { "declaration", "definition", "readonly", "static", "deprecated", "abstract", "async", "modification", "documentation", "defaultLibrary" },
          tokenTypes = { "namespace", "type", "class", "enum", "interface", "struct", "typeParameter", "parameter", "variable", "property", "enumMember", "event", "function", "method", "macro", "keyword", "modifier", "comment", "string", "number", "regexp", "operator", "decorator" }
        },
        signatureHelp = {
          dynamicRegistration = false,
          signatureInformation = {
            activeParameterSupport = true,
            documentationFormat = { "markdown", "plaintext" },
            parameterInformation = {
              labelOffsetSupport = true
            }
          }
        },
        synchronization = {
          didSave = true,
          dynamicRegistration = false,
          willSave = true,
          willSaveWaitUntil = true
        },
        typeDefinition = {
          linkSupport = true
        }
      },
      window = {
        showDocument = {
          support = true
        },
        showMessage = {
          messageActionItem = {
            additionalPropertiesSupport = false
          }
        },
        workDoneProgress = true
      },
      workspace = {
        applyEdit = true,
        configuration = true,
        didChangeWatchedFiles = {
          dynamicRegistration = false,
          relativePatternSupport = true
        },
        semanticTokens = {
          refreshSupport = true
        },
        symbol = {
          dynamicRegistration = false,
          hierarchicalWorkspaceSymbolSupport = 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 }
          }
        },
        workspaceEdit = {
          resourceOperations = { "rename", "create", "delete" }
        },
        workspaceFolders = true
      }
    },
    clientInfo = {
      name = "Neovim",
      version = "0.9.2"
    },
    processId = 82064,
    rootPath = "/Users/erik/git/contextive-demo-go-common",
    rootUri = "file:///Users/erik/git/contextive-demo-go-common",
    trace = "off",
    workspaceFolders = { {
        name = "/Users/erik/git/contextive-demo-go-common",
        uri = "file:///Users/erik/git/contextive-demo-go-common"
      } }
  }
}
[DEBUG][2023-10-22 20:29:16] .../vim/lsp/rpc.lua:387	"rpc.receive"	{
  id = 1,
  jsonrpc = "2.0",
  result = {
    capabilities = {
      completionProvider = vim.empty_dict(),
      experimental = vim.empty_dict(),
      hoverProvider = vim.empty_dict(),
      textDocumentSync = {
        change = 1,
        openClose = true,
        save = {
          includeText = true
        }
      },
      workspace = {
        fileOperations = vim.empty_dict(),
        workspaceFolders = {
          changeNotifications = true,
          supported = true
        }
      }
    },
    serverInfo = {
      name = "Contextive.LanguageServer",
      version = "1.10.2"
    }
  }
}
[DEBUG][2023-10-22 20:29:16] .../vim/lsp/rpc.lua:284	"rpc.send"	{
  jsonrpc = "2.0",
  method = "initialized",
  params = vim.empty_dict()
}
[DEBUG][2023-10-22 20:29:16] .../vim/lsp/rpc.lua:284	"rpc.send"	{
  jsonrpc = "2.0",
  method = "workspace/didChangeConfiguration",
  params = {
    settings = {
      contextive = {
        path = "/Users/erik/git/contextive-demo-go-common/.contextive.yml"
      }
    }
  }
}
[DEBUG][2023-10-22 20:29:16] .../vim/lsp/rpc.lua:284	"rpc.send"	{
  jsonrpc = "2.0",
  method = "workspace/configuration",
  params = {
    settings = {
      contextive = {
        path = "/Users/erik/git/contextive-demo-go-common/.contextive.yml"
      }
    }
  }
}
[DEBUG][2023-10-22 20:29:16] .../vim/lsp/rpc.lua:284	"rpc.send"	{
  jsonrpc = "2.0",
  method = "workspace/didChangeConfiguration",
  params = {
    settings = {
      contextive = {
        path = "/Users/erik/git/contextive-demo-go-common/.contextive.yml"
      }
    }
  }
}
[DEBUG][2023-10-22 20:29:16] .../vim/lsp/rpc.lua:284	"rpc.send"	{
  jsonrpc = "2.0",
  method = "workspace/didChangeConfiguration",
  params = {
    settings = {
      contextive = {
        path = "/Users/erik/git/contextive-demo-go-common/.contextive.yml"
      }
    }
  }
}
[INFO][2023-10-22 20:29:16] .../lua/vim/lsp.lua:1344	"LSP[contextive]"	"server_capabilities"	{
  server_capabilities = {
    completionProvider = vim.empty_dict(),
    experimental = vim.empty_dict(),
    hoverProvider = vim.empty_dict(),
    textDocumentSync = {
      change = 1,
      openClose = true,
      save = {
        includeText = true
      }
    },
    workspace = {
      fileOperations = vim.empty_dict(),
      workspaceFolders = {
        changeNotifications = true,
        supported = true
      }
    }
  }
}
[DEBUG][2023-10-22 20:29:16] .../vim/lsp/rpc.lua:284	"rpc.send"	{
  jsonrpc = "2.0",
  method = "textDocument/didOpen",
  params = {
    textDocument = {
      languageId = "go",
      text = 'package sample\n\nfunc Sample() string {\n\treturn "This is a great sample!"\n}\n\n',
      uri = "file:///Users/erik/git/contextive-demo-go-common/sample.go",
      version = 0
    }
  }
}
[DEBUG][2023-10-22 20:29:16] .../vim/lsp/rpc.lua:387	"rpc.receive"	{
  jsonrpc = "2.0",
  method = "window/logMessage",
  params = {
    message = "Starting Contextive.LanguageServer v1.10.2...",
    type = 3
  }
}
[INFO][2023-10-22 20:29:16] ...lsp/handlers.lua:539	"Starting Contextive.LanguageServer v1.10.2..."
[DEBUG][2023-10-22 20:29:16] .../vim/lsp/rpc.lua:387	"rpc.receive"	{
  jsonrpc = "2.0",
  method = "window/logMessage",
  params = {
    message = "Loading contextive from /Users/erik/git/contextive-demo-go-common/.contextive/definitions.yml...",
    type = 3
  }
}
[INFO][2023-10-22 20:29:16] ...lsp/handlers.lua:539	"Loading contextive from /Users/erik/git/contextive-demo-go-common/.contextive/definitions.yml..."
[DEBUG][2023-10-22 20:29:16] .../vim/lsp/rpc.lua:387	"rpc.receive"	{
  jsonrpc = "2.0",
  method = "window/logMessage",
  params = {
    message = "Error loading definitions: Definitions file not found.",
    type = 3
  }
}
[INFO][2023-10-22 20:29:16] ...lsp/handlers.lua:539	"Error loading definitions: Definitions file not found."
[DEBUG][2023-10-22 20:29:16] .../vim/lsp/rpc.lua:387	"rpc.receive"	{
  jsonrpc = "2.0",
  method = "window/showMessage",
  params = {
    message = "Error loading definitions: Definitions file not found.",
    type = 2
  }
}

[chrissimon-au]

Thanks for sharing the log - this is very interesting! I would have expected a rpc.receive with method workspace/configuration but there is none - so for some reason the OmniSharp server has decided not to issue a configuration request. But this does explain the issue you were having because without that request, you'd never get a response, and as a result you'd experience the issue you found which required you to introduce the mutable for capturing the value from the didChangeConfiguration notification.

I'll review the vscode protocol logs and look for what the differences are prior to that point... probably some subtlety in the initialize request...

[erikjuhani]

Thanks for sharing the log - this is very interesting! I would have expected a rpc.receive with method workspace/configuration but there is none - so for some reason the OmniSharp server has decided not to issue a configuration request. But this does explain the issue you were having because without that request, you'd never get a response, and as a result you'd experience the issue you found which required you to introduce the mutable for capturing the value from the didChangeConfiguration notification.

I'll review the vscode protocol logs and look for what the differences are prior to that point... probably some subtlety in the initialize request...

No problem! Very interesting indeed, and well yeah that explains it on why defining the settings did not work as expected.

[chrissimon-au]

Hi @erikjuhani, I've had a chance to dig into this a bit further and I think I see why it's hard to diagnose... I think we're dealing with the intersection of two bugs - one in neovim and one in OmniSharp 🤣

OmniSharp Bug

The neovim lsp client does not send the DidChangeConfiguration capability - only the Configuration capability. While in theory this should be sufficient, for some reason the omnisharp server is conflating the Configuration capability and the DidChangeConfiguration capability, and only sending a workspace/configuration request if the client advertised the DidChangeConfiguration capability - it should send the workspace/configuration request as long as the Configuration capability has been advertised.

I'm working through this to confirm, and once I do I'll raise a bug with the OmniSharp library.

NeoVim Bug

While it's technically correct that the nevom lsp client doesn't advertise the capability (because it has no support for monitoring the configuration settings and sending a notification when they are modified), it DOES actually send the DidChangeConfiguration notification after initialisation if there are any settings configured (after 0.8, as you noted above). This is probably not technically correct in the spec, as the server would be entitled to ignore that notification given that the capability was not advertised, so it really should be advertising the capability.

Solutions

While fixes to upstream of either bug would resolve the situation, there are some potential ways to resolve it from the contextive end:

1. Migrate to an IDE agnostic config approach, as per your suggestion in Editor agnostic configuration #54 - this is probably end-goal, but needs some careful thought for reasons I'll shortly explain on that ticket
2. Abandon use of the workspace/configuration request, and rely solely on the didChangeConfiguration notification. This has some pros and cons - pros, it would work for both vscode and neovim, cons are it might limit compatibility with other clients that only support workspace/configuration. I think I initially adopted workspace/configuration thinking it was more likely to be supported by clients. While this is the high level approach your PR takes, if we are going to do this I'd prefer to note use the mutable state management and the configGetter delegates and just pass the values directly to the definitions file handling.
3. Workaround the omnisharp bug by bypassing the convenience methods and directly sending the workspace/configuration request. This is probably the hackiest but shortest path to getting a version that works until omnisharp resolves their issue.

Given that option 1 is 'end-goal', I'm thinking for now I'll go with option 3 as moving to 1 will involve a pretty big adjustment of the whole config approach, so not much point in doing option 2.

Any thoughts?

[chrissimon-au]

Have reported with OmniSharp here: OmniSharp/csharp-language-server-protocol#1101

[chrissimon-au]

Closing in favour of #58