fix(language-server): support LSP clients that only support workspace/configuration (#58)

* fix(language-server): support LSP clients that only support `workspace/configuration`.

See OmniSharp/csharp-language-server-protocol#1101 for the upstream bug that necessitates this.  We can revert to unscoped configuration when that is resolved.

* docs(neovim): update instructions to illustrate setting a custom definitions file location

Author: chrissimon-au

README.md

@@ -50,7 +50,7 @@ The following list of features is a draft proposal of the vision at the start of
   * [ ] Eclipse
   * [ ] NetBeans
   * [ ] JetBrains
-  * [ ] vim
+  * [x] neovim
   * [ ] emacs
 * [ ] Code-editing Features
   * [x] Show the term definitions & usage examples when hovering over the word in the editor 
@@ -128,6 +128,7 @@ lspconfig_configs.contextive = {
   default_config = {
     cmd = { "Contextive.LanguageServer" },
     root_dir = lspconfig.util.root_pattern('.contextive', '.git'),
+    --settings={contextive={path="./path/to/definitions.yml"} -- uncomment this line to nominate a custom definitions.yml file location
   },
 }
 

src/language-server/Contextive.LanguageServer.Tests/CompletionTests.fs

@@ -30,7 +30,7 @@ let completionTests =
                   $"Given {fileName} contextive, in document {text} at position {position} respond with expected completion list " {
                   let config =
                       [ Workspace.optionsBuilder <| Path.Combine("fixtures", "completion_tests")
-                        ConfigurationSection.contextivePathOptionsBuilder $"{fileName}.yml" ]
+                        ConfigurationSection.contextivePathBuilder $"{fileName}.yml" ]
 
                   use! client = TestClient(config) |> init
 

src/language-server/Contextive.LanguageServer.Tests/ConfigurationTests.fs

@@ -13,7 +13,7 @@ let definitionsTests =
         [ testAsync "Can receive configuration value" {
               let config =
                   [ Workspace.optionsBuilder <| Path.Combine("fixtures", "completion_tests")
-                    ConfigurationSection.contextivePathOptionsBuilder "one.yml" ]
+                    ConfigurationSection.contextivePathBuilder "one.yml" ]
 
               use! client = TestClient(config) |> init
 
@@ -29,17 +29,32 @@ let definitionsTests =
 
               let config =
                   [ Workspace.optionsBuilder <| Path.Combine("fixtures", "completion_tests")
-                    ConfigurationSection.contextivePathLoaderOptionsBuilder pathLoader ]
+                    ConfigurationSection.contextivePathLoaderBuilder pathLoader ]
 
               use! client = TestClient(config) |> init
 
               path <- "two.yml"
-              ConfigurationSection.didChange client path
+              ConfigurationSection.didChangePath client path
 
               let! labels = Completion.getCompletionLabels client
 
               test <@ (labels, Fixtures.Two.expectedCompletionLabels) ||> Seq.compareWith compare = 0 @>
 
           }
 
+          testAsync "Can handle client that doesn't support didChangeConfiguration" {
+              let mutable path = "one.yml"
+
+              let config =
+                  [ Workspace.optionsBuilder <| Path.Combine("fixtures", "completion_tests")
+                    ConfigurationSection.contextivePathBuilder path ]
+
+              use! client = TestClient(config) |> init
+
+              let! labels = Completion.getCompletionLabels client
+
+              test <@ (labels, Fixtures.One.expectedCompletionLabels) ||> Seq.compareWith compare = 0 @>
+
+          }
+
           ]

src/language-server/Contextive.LanguageServer.Tests/DefinitionsTests.fs

@@ -163,21 +163,21 @@ let definitionsTests =
 
                   let config =
                       [ Workspace.optionsBuilder <| Path.Combine("fixtures", "completion_tests")
-                        ConfigurationSection.contextivePathLoaderOptionsBuilder pathLoader ]
+                        ConfigurationSection.contextivePathLoaderBuilder pathLoader ]
 
                   use! client = TestClient(config) |> init
 
                   let! termsWhenValidAtStart = Completion.getCompletionLabels client
                   test <@ (termsWhenValidAtStart, Fixtures.One.expectedCompletionLabels) ||> compareList = 0 @>
 
                   path <- $"{fileName}.yml"
-                  ConfigurationSection.didChange client path
+                  ConfigurationSection.didChangePath client path
 
                   let! termsWhenInvalid = Completion.getCompletionLabels client
                   test <@ Seq.length termsWhenInvalid = 0 @>
 
                   path <- validPath
-                  ConfigurationSection.didChange client path
+                  ConfigurationSection.didChangePath client path
 
                   let! termsWhenValidAtEnd = Completion.getCompletionLabels client
                   test <@ (termsWhenValidAtEnd, Fixtures.One.expectedCompletionLabels) ||> compareList = 0 @>

src/language-server/Contextive.LanguageServer.Tests/Helpers/ConfigurationSection.fs

@@ -9,7 +9,7 @@ open OmniSharp.Extensions.LanguageServer.Protocol.Client
 open OmniSharp.Extensions.LanguageServer.Protocol.Workspace
 open System.Threading.Tasks
 
-let jTokenFromMap values =
+let private jTokenFromMap values =
     let configValue = JObject()
 
     values
@@ -38,27 +38,50 @@ let private createHandler section configValuesLoader =
         else
             Task.FromResult(configSectionResultFromMap <| Map [])
 
-let optionsBuilder section configValuesLoader (b: LanguageClientOptions) =
+let configurationHandlerBuilder section configValuesLoader (b: LanguageClientOptions) =
     let handler = createHandler section configValuesLoader
-
+    b.OnConfiguration(handler) |> ignore
     b
-        .WithCapability(Capabilities.DidChangeConfigurationCapability(DynamicRegistration = true))
-        .OnConfiguration(handler)
+
+let private didChangeConfigurationBuilder (b: LanguageClientOptions) =
+    b.WithCapability(Capabilities.DidChangeConfigurationCapability(DynamicRegistration = true))
     |> ignore
 
     b
 
-type PathLoader = unit -> obj
-
-let mapLoader (pathLoader: PathLoader) () = Map[("path", pathLoader ())]
-
-let contextivePathLoaderOptionsBuilder (pathLoader: PathLoader) =
-    optionsBuilder "contextive" <| mapLoader pathLoader
-
-let contextivePathOptionsBuilder path =
-    contextivePathLoaderOptionsBuilder (fun () -> path)
-
-let didChange (client: ILanguageClient) path =
+type ValueLoader = unit -> obj
+
+let private mapLoader (key: string) (valueLoader: ValueLoader) () = Map[(key, valueLoader ())]
+
+let private mapPathLoader = mapLoader "path"
+
+let private contextivePathConfigurationHandlerBuilder (pathLoader: ValueLoader) =
+    configurationHandlerBuilder "contextive" <| mapPathLoader pathLoader
+
+/// <summary>
+///     <para>Use when you have a setting value that needs to change during the test.</para>
+///     <para>The test client will support `workspace/configuration` AND `workspace/didChangeConfiguration`.</para>
+///     <para>When a `workspace/configuration` request is received, the `pathLoader` function will be invoked to get the current value.</para>
+///     <para>To trigger a `workspace/didChangeConfiguration` notification, use <see cref="didChangePath">didChangePath</see>.</para>
+/// </summary>
+let contextivePathLoaderBuilder (pathLoader: ValueLoader) =
+    contextivePathConfigurationHandlerBuilder pathLoader
+    >> didChangeConfigurationBuilder
+
+/// <summary>
+///     <para>Use when you have a fixed setting value.</para>
+///     <para>The test client will ONLY support `workspace/configuration` and will always supply this fixed value</para>
+///     <para>Using <see cref="didChangePath">didChangePath</see> will have no impact as the server will not support the `workspace/didChangeConfiguration` notification</para>
+/// </summary>
+let contextivePathBuilder path =
+    contextivePathConfigurationHandlerBuilder (fun () -> path)
+
+/// <summary>
+///     <para>Trigger a `workspace/didChangeConfiguration` notification with the new value.</para>
+///     <para>This will only work if the TestClient was created with the <see cref="contextivePathLoaderBuilder">contextivePathLoaderBuilder</see>.</para>
+///     <para>Ensure this method is invoked with the same path value as will be returned from the `pathLoader` registered with the TestClient.</para>
+/// </summary>
+let didChangePath (client: ILanguageClient) path =
     let setting = jTokenFromMap <| Map[("path", path)]
     let configSection = jTokenFromMap <| Map[("contextive", setting)]
 

src/language-server/Contextive.LanguageServer.Tests/HoverTests.fs

@@ -37,7 +37,7 @@ let hoverTests =
                   $"Given definitions file '{fileName}' and file contents '{multiLineToSingleLine text}', server responds to hover request at Position {position} with '{expectedTerm}'" {
                   let config =
                       [ Workspace.optionsBuilder <| Path.Combine("fixtures", "completion_tests")
-                        ConfigurationSection.contextivePathOptionsBuilder $"{fileName}.yml" ]
+                        ConfigurationSection.contextivePathBuilder $"{fileName}.yml" ]
 
                   use! client = TestClient(config) |> init
 
@@ -89,7 +89,7 @@ let hoverTests =
 
                   let config =
                       [ Workspace.optionsBuilder <| Path.Combine("fixtures", "completion_tests")
-                        ConfigurationSection.contextivePathOptionsBuilder $"{fileName}.yml" ]
+                        ConfigurationSection.contextivePathBuilder $"{fileName}.yml" ]
 
                   use! client = TestClient(config) |> init
 
@@ -129,7 +129,7 @@ let hoverTests =
 
                   let config =
                       [ Workspace.optionsBuilder <| Path.Combine("fixtures", "completion_tests")
-                        ConfigurationSection.contextivePathOptionsBuilder $"{fileName}.yml" ]
+                        ConfigurationSection.contextivePathBuilder $"{fileName}.yml" ]
 
                   use! client = TestClient(config) |> init
 

src/language-server/Contextive.LanguageServer.Tests/InitializationTests.fs

@@ -28,38 +28,34 @@ let initializationTests =
 
               let config =
                   [ Workspace.optionsBuilder ""
-                    ConfigurationSection.contextivePathOptionsBuilder pathValue ]
+                    ConfigurationSection.contextivePathBuilder pathValue ]
 
               let! (client, reply) = TestClient(config) |> initAndWaitForReply
 
               test <@ client.ClientSettings.Capabilities.Workspace.Configuration.IsSupported @>
-              test <@ client.ClientSettings.Capabilities.Workspace.DidChangeConfiguration.IsSupported @>
 
               test <@ (defaultArg reply "").Contains(pathValue) @>
           }
 
           testAsync "Server loads contextive file from absolute location without workspace" {
               let pathValue = Guid.NewGuid().ToString()
 
-              let config =
-                  [ ConfigurationSection.contextivePathOptionsBuilder $"/tmp/{pathValue}" ]
+              let config = [ ConfigurationSection.contextivePathBuilder $"/tmp/{pathValue}" ]
 
               let! (client, reply) = TestClient(config) |> initAndWaitForReply
 
               test <@ client.ClientSettings.Capabilities.Workspace.Configuration.IsSupported @>
-              test <@ client.ClientSettings.Capabilities.Workspace.DidChangeConfiguration.IsSupported @>
 
               test <@ (defaultArg reply "").Contains(pathValue) @>
           }
 
           testAsync "Server does NOT load contextive file from relative location without workspace" {
               let pathValue = Guid.NewGuid().ToString()
-              let config = [ ConfigurationSection.contextivePathOptionsBuilder pathValue ]
+              let config = [ ConfigurationSection.contextivePathBuilder pathValue ]
 
               let! (client, reply) = TestClientWithCustomInitWait(config, Some pathValue) |> initAndWaitForReply
 
               test <@ client.ClientSettings.Capabilities.Workspace.Configuration.IsSupported @>
-              test <@ client.ClientSettings.Capabilities.Workspace.DidChangeConfiguration.IsSupported @>
 
               test
                   <@
@@ -71,7 +67,7 @@ let initializationTests =
           testAsync "Server loads contextive file from default location when no configuration supplied" {
               let config =
                   [ Workspace.optionsBuilder ""
-                    ConfigurationSection.optionsBuilder "dummySection" (fun () -> Map []) ]
+                    ConfigurationSection.configurationHandlerBuilder "dummySection" (fun () -> Map []) ]
 
               let defaultPath = ".contextive/definitions.yml"
 

src/language-server/Contextive.LanguageServer.Tests/SurveyTests.fs

@@ -48,7 +48,7 @@ let initializationTests =
               let config =
                   [ showMessageRequestHandlerBuilder <| handler messageAwaiter null
                     Workspace.optionsBuilder ""
-                    ConfigurationSection.contextivePathOptionsBuilder pathValue ]
+                    ConfigurationSection.contextivePathBuilder pathValue ]
 
               let! (client, reply) = TestClient(config) |> initAndWaitForReply
 
@@ -75,7 +75,7 @@ let initializationTests =
               let config =
                   [ showMessageRequestHandlerBuilder <| handler messageAwaiter null
                     Workspace.optionsBuilder ""
-                    ConfigurationSection.contextivePathOptionsBuilder pathValue ]
+                    ConfigurationSection.contextivePathBuilder pathValue ]
 
               let! (client, reply) = TestClient(config) |> initAndWaitForReply
 
@@ -102,7 +102,7 @@ let initializationTests =
                   [ showMessageRequestHandlerBuilder <| handler messageAwaiter response
                     showDocumentRequestHandlerBuilder <| handler showDocAwaiter showDocResponse
                     Workspace.optionsBuilder ""
-                    ConfigurationSection.contextivePathOptionsBuilder pathValue ]
+                    ConfigurationSection.contextivePathBuilder pathValue ]
 
               let! (client, reply) = TestClient(config) |> initAndWaitForReply
 

src/language-server/Contextive.LanguageServer.Tests/TextDocumentTests.fs

@@ -52,7 +52,7 @@ let textDocumentTests =
           testAsync "Server supports full sync" {
               let config =
                   [ Workspace.optionsBuilder <| Path.Combine("fixtures", "completion_tests")
-                    ConfigurationSection.contextivePathOptionsBuilder $"one.yml" ]
+                    ConfigurationSection.contextivePathBuilder $"one.yml" ]
 
               use! client = TestClient(config) |> init
               test <@ client.ServerSettings.Capabilities.TextDocumentSync.Options.Change = TextDocumentSyncKind.Full @>

src/language-server/Contextive.LanguageServer.Tests/WatchedFilesTests.fs

@@ -36,7 +36,7 @@ let watchedFileTests =
 
               let config =
                   [ Workspace.optionsBuilder <| Path.Combine("fixtures", "completion_tests")
-                    ConfigurationSection.contextivePathOptionsBuilder $"one.yml"
+                    ConfigurationSection.contextivePathBuilder $"one.yml"
                     WatchedFiles.optionsBuilder registrationAwaiter ]
 
               let! client = TestClient(config) |> init
@@ -62,7 +62,7 @@ let watchedFileTests =
 
                   let config =
                       [ Workspace.optionsBuilder <| Path.Combine("fixtures", "completion_tests")
-                        ConfigurationSection.contextivePathLoaderOptionsBuilder pathLoader
+                        ConfigurationSection.contextivePathLoaderBuilder pathLoader
                         WatchedFiles.optionsBuilder registrationAwaiter ]
 
                   let! client = TestClient(config) |> init
@@ -72,7 +72,7 @@ let watchedFileTests =
                   ConditionAwaiter.clear registrationAwaiter 500
 
                   definitionsFile <- newDefinitionsFile
-                  ConfigurationSection.didChange client definitionsFile
+                  ConfigurationSection.didChangePath client definitionsFile
 
                   let! secondRegistrationMsg =
                       ConditionAwaiter.waitFor
@@ -113,7 +113,7 @@ let watchedFileTests =
 
               let config =
                   [ Workspace.optionsBuilder relativePath
-                    ConfigurationSection.contextivePathOptionsBuilder definitionsFile ]
+                    ConfigurationSection.contextivePathBuilder definitionsFile ]
 
               let! client = TestClient(config) |> init
 
@@ -153,7 +153,7 @@ let watchedFileTests =
 
               let config =
                   [ Workspace.optionsBuilder relativePath
-                    ConfigurationSection.contextivePathOptionsBuilder definitionsFile ]
+                    ConfigurationSection.contextivePathBuilder definitionsFile ]
 
               let! client = TestClient(config) |> init
 

src/language-server/Contextive.LanguageServer/Server.fs

@@ -30,7 +30,10 @@ let private getConfig (s: ILanguageServer) section key =
         Log.Logger.Information $"Getting {section} {key} config..."
 
         let! config =
-            s.Configuration.GetConfiguration(ConfigurationItem(Section = configSection))
+            // We need to use `GetScopedConfiguration` because of https://github.com/OmniSharp/csharp-language-server-protocol/issues/1101
+            // We can revert to `GetConfiguration` when that bug is fixed.
+            s.Configuration.GetScopedConfiguration("file:///", System.Threading.CancellationToken.None)
+            //s.Configuration.GetConfiguration(ConfigurationItem(Section = configSection))
             |> Async.AwaitTask
 
         let configValue = config.GetSection(section).Item(key)