Skip to content

Commit

Permalink
fix(language-server): support LSP clients that only support `workspac…
Browse files Browse the repository at this point in the history
…e/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
  • Loading branch information
chrissimon-au authored Nov 13, 2023
1 parent 62fa42f commit 7e06396
Show file tree
Hide file tree
Showing 11 changed files with 83 additions and 45 deletions.
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
},
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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 @>

}

]
Original file line number Diff line number Diff line change
Expand Up @@ -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 @>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)]

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
<@
Expand All @@ -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"

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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

Expand All @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 @>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -72,7 +72,7 @@ let watchedFileTests =
ConditionAwaiter.clear registrationAwaiter 500

definitionsFile <- newDefinitionsFile
ConfigurationSection.didChange client definitionsFile
ConfigurationSection.didChangePath client definitionsFile

let! secondRegistrationMsg =
ConditionAwaiter.waitFor
Expand Down Expand Up @@ -113,7 +113,7 @@ let watchedFileTests =

let config =
[ Workspace.optionsBuilder relativePath
ConfigurationSection.contextivePathOptionsBuilder definitionsFile ]
ConfigurationSection.contextivePathBuilder definitionsFile ]

let! client = TestClient(config) |> init

Expand Down Expand Up @@ -153,7 +153,7 @@ let watchedFileTests =

let config =
[ Workspace.optionsBuilder relativePath
ConfigurationSection.contextivePathOptionsBuilder definitionsFile ]
ConfigurationSection.contextivePathBuilder definitionsFile ]

let! client = TestClient(config) |> init

Expand Down
5 changes: 4 additions & 1 deletion src/language-server/Contextive.LanguageServer/Server.fs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit 7e06396

Please sign in to comment.