Skip to content

Commit 3dd94c7

Browse files
bjorkstrommbjorkstromm
authored
Merge pull request #163 from OmniSharp/fix/text-options-defaults
Changes for #162 correct how text document sync settings are configured
2 parents d997627 + 155948f commit 3dd94c7

File tree

6 files changed

+143
-43
lines changed

6 files changed

+143
-43
lines changed

‎src/Protocol/Client/Capabilities/SynchronizationCapability.cs

+7-1
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,13 @@
55

66
namespace OmniSharp.Extensions.LanguageServer.Protocol.Client.Capabilities
77
{
8-
public class SynchronizationCapability : DynamicCapability, ConnectedCapability<IDidChangeTextDocumentHandler>, ConnectedCapability<IDidCloseTextDocumentHandler>, ConnectedCapability<IDidOpenTextDocumentHandler>, ConnectedCapability<IDidSaveTextDocumentHandler>, ConnectedCapability<IWillSaveTextDocumentHandler>, ConnectedCapability<IWillSaveWaitUntilTextDocumentHandler>
8+
public class SynchronizationCapability : DynamicCapability,
9+
ConnectedCapability<IDidChangeTextDocumentHandler>,
10+
ConnectedCapability<IDidCloseTextDocumentHandler>,
11+
ConnectedCapability<IDidOpenTextDocumentHandler>,
12+
ConnectedCapability<IDidSaveTextDocumentHandler>,
13+
ConnectedCapability<IWillSaveTextDocumentHandler>,
14+
ConnectedCapability<IWillSaveWaitUntilTextDocumentHandler>
915
{
1016
/// <summary>
1117
/// The client supports sending will save notifications.

‎src/Protocol/Server/Capabilities/TextDocumentSyncOptions.cs

+14-6
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
using System;
12
using System.Collections.Generic;
23
using System.Linq;
34
using Newtonsoft.Json;
@@ -38,17 +39,24 @@ public class TextDocumentSyncOptions : ITextDocumentSyncOptions
3839

3940
public static TextDocumentSyncOptions Of(IEnumerable<ITextDocumentSyncOptions> options)
4041
{
41-
return new TextDocumentSyncOptions() {
42-
OpenClose = options.Any(z => z.OpenClose),
43-
Change = options
42+
var change = TextDocumentSyncKind.None;
43+
if (options.Any(x => x.Change != TextDocumentSyncKind.None))
44+
{
45+
change = options
4446
.Where(x => x.Change != TextDocumentSyncKind.None)
45-
.Min(z => z.Change),
47+
.Min(z => z.Change);
48+
}
49+
return new TextDocumentSyncOptions()
50+
{
51+
OpenClose = options.Any(z => z.OpenClose),
52+
Change = change,
4653
WillSave = options.Any(z => z.WillSave),
4754
WillSaveWaitUntil = options.Any(z => z.WillSaveWaitUntil),
48-
Save = new SaveOptions() {
55+
Save = new SaveOptions()
56+
{
4957
IncludeText = options.Any(z => z.Save?.IncludeText == true)
5058
}
51-
};
59+
};
5260
}
5361
}
5462
}

‎src/Server/ClientCapabilityProvider.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ public bool HasStaticHandler<T>(Supports<T> capability)
3030
.Where(x => x.GetTypeInfo().IsGenericType && x.GetTypeInfo().GetGenericTypeDefinition() == typeof(ConnectedCapability<>))
3131
.Select(x => x.GetTypeInfo().GetGenericArguments()[0].GetTypeInfo());
3232

33-
return handlerTypes.All(_collection.ContainsHandler);
33+
return handlerTypes.Any(_collection.ContainsHandler);
3434
}
3535

3636
public IOptionsGetter GetStaticOptions<T>(Supports<T> capability)

‎src/Server/LanguageServer.cs

+15-7
Original file line numberDiff line numberDiff line change
@@ -313,7 +313,8 @@ private IDisposable RegisterHandlers(LspHandlerDescriptorDisposable handlerDispo
313313
{
314314
var registrations = handlerDisposable.Descriptors
315315
.Where(d => d.AllowsDynamicRegistration)
316-
.Select(d => new Registration() {
316+
.Select(d => new Registration()
317+
{
317318
Id = d.Id.ToString(),
318319
Method = d.Method,
319320
RegisterOptions = d.RegistrationOptions
@@ -433,13 +434,20 @@ async Task<InitializeResult> IRequestHandler<InitializeParams, InitializeResult>
433434

434435
if (ccp.HasStaticHandler(textDocumentCapabilities.Synchronization))
435436
{
436-
var textDocumentSyncKind = _collection.ContainsHandler(typeof(IDidChangeTextDocumentHandler))
437-
? _collection
437+
var textDocumentSyncKind = TextDocumentSyncKind.None;
438+
if (_collection.ContainsHandler(typeof(IDidChangeTextDocumentHandler)))
439+
{
440+
var kinds = _collection
438441
.Select(x => x.Handler)
439442
.OfType<IDidChangeTextDocumentHandler>()
440-
.Where(x => x.GetRegistrationOptions()?.SyncKind != TextDocumentSyncKind.None)
441-
.Min(z => z.GetRegistrationOptions()?.SyncKind)
442-
: TextDocumentSyncKind.None;
443+
.Select(x => x.GetRegistrationOptions()?.SyncKind ?? TextDocumentSyncKind.None)
444+
.Where(x => x != TextDocumentSyncKind.None)
445+
.ToArray();
446+
if (kinds.Any())
447+
{
448+
textDocumentSyncKind = kinds.Min(z => z);
449+
}
450+
}
443451

444452
if (_clientVersion == ClientVersion.Lsp2)
445453
{
@@ -449,7 +457,7 @@ async Task<InitializeResult> IRequestHandler<InitializeParams, InitializeResult>
449457
{
450458
serverCapabilities.TextDocumentSync = new TextDocumentSyncOptions()
451459
{
452-
Change = textDocumentSyncKind ?? TextDocumentSyncKind.None,
460+
Change = TextDocumentSyncKind.None,
453461
OpenClose = _collection.ContainsHandler(typeof(IDidOpenTextDocumentHandler)) || _collection.ContainsHandler(typeof(IDidCloseTextDocumentHandler)),
454462
Save = _collection.ContainsHandler(typeof(IDidSaveTextDocumentHandler)) ?
455463
new SaveOptions() { IncludeText = true /* TODO: Make configurable */ } :

‎test/Lsp.Tests/ClientCapabilityProviderTests.cs

+67-8
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,8 @@ public void Should_AllowSupportedCapabilities(IJsonRpcHandler handler, object in
3636

3737
public static IEnumerable<object[]> AllowSupportedCapabilities()
3838
{
39-
return GetItems(Capabilities, type => {
39+
return GetItems(Capabilities, type =>
40+
{
4041
var handlerTypes = GetHandlerTypes(type);
4142
var handler = Substitute.For(handlerTypes.ToArray(), new object[0]);
4243
return new[] { handler, Activator.CreateInstance(typeof(Supports<>).MakeGenericType(type), true, Activator.CreateInstance(type)) };
@@ -56,7 +57,8 @@ public void Should_AllowUnsupportedCapabilities(IJsonRpcHandler handler, object
5657

5758
public static IEnumerable<object[]> AllowUnsupportedCapabilities()
5859
{
59-
return GetItems(Capabilities, type => {
60+
return GetItems(Capabilities, type =>
61+
{
6062
var handlerTypes = GetHandlerTypes(type);
6163
var handler = Substitute.For(handlerTypes.ToArray(), new object[0]);
6264
return new[] { handler, Activator.CreateInstance(typeof(Supports<>).MakeGenericType(type), false) };
@@ -104,7 +106,8 @@ public void Should_AllowNullSupportedCapabilities(IJsonRpcHandler handler, objec
104106

105107
public static IEnumerable<object[]> AllowNullSupportsCapabilities()
106108
{
107-
return GetItems(Capabilities, type => {
109+
return GetItems(Capabilities, type =>
110+
{
108111
var handlerTypes = GetHandlerTypes(type);
109112
var handler = Substitute.For(handlerTypes.ToArray(), new object[0]);
110113
return new[] { handler, Activator.CreateInstance(typeof(Supports<>).MakeGenericType(type), true) };
@@ -125,7 +128,8 @@ public void Should_DisallowDynamicSupportedCapabilities(IJsonRpcHandler handler,
125128

126129
public static IEnumerable<object[]> DisallowDynamicSupportsCapabilities()
127130
{
128-
return GetItems(Capabilities, type => {
131+
return GetItems(Capabilities, type =>
132+
{
129133
var handlerTypes = GetHandlerTypes(type);
130134
var handler = Substitute.For(handlerTypes.ToArray(), new object[0]);
131135
var capability = Activator.CreateInstance(type);
@@ -145,12 +149,16 @@ public void Should_Handle_Mixed_Capabilities()
145149

146150
var collection = new HandlerCollection(SupportedCapabilitiesFixture.AlwaysTrue, new TextDocumentIdentifiers()) { textDocumentSyncHandler, codeActionHandler, definitionHandler, typeDefinitionHandler };
147151
var provider = new ClientCapabilityProvider(collection);
148-
var capabilities = new ClientCapabilities() {
149-
TextDocument = new TextDocumentClientCapabilities() {
150-
CodeAction = new Supports<CodeActionCapability>(true, new CodeActionCapability() {
152+
var capabilities = new ClientCapabilities()
153+
{
154+
TextDocument = new TextDocumentClientCapabilities()
155+
{
156+
CodeAction = new Supports<CodeActionCapability>(true, new CodeActionCapability()
157+
{
151158
DynamicRegistration = false,
152159
}),
153-
TypeDefinition = new Supports<TypeDefinitionCapability>(true, new TypeDefinitionCapability() {
160+
TypeDefinition = new Supports<TypeDefinitionCapability>(true, new TypeDefinitionCapability()
161+
{
154162
DynamicRegistration = true,
155163
})
156164
}
@@ -161,6 +169,57 @@ public void Should_Handle_Mixed_Capabilities()
161169
provider.HasStaticHandler(capabilities.TextDocument.TypeDefinition).Should().BeFalse();
162170
}
163171

172+
[Fact]
173+
public void GH162_TextDocumentSync_Should_Work_Without_WillSave_Or_WillSaveWaitUntil()
174+
{
175+
var textDocumentSyncHandler = TextDocumentSyncHandlerExtensions.With(DocumentSelector.ForPattern("**/*.cs"), "csharp");
176+
177+
var collection = new HandlerCollection(SupportedCapabilitiesFixture.AlwaysTrue, new TextDocumentIdentifiers()) { textDocumentSyncHandler };
178+
var provider = new ClientCapabilityProvider(collection);
179+
var capabilities = new ClientCapabilities()
180+
{
181+
TextDocument = new TextDocumentClientCapabilities()
182+
{
183+
Synchronization = new SynchronizationCapability()
184+
{
185+
DidSave = true,
186+
DynamicRegistration = false,
187+
WillSave = true,
188+
WillSaveWaitUntil = true
189+
},
190+
}
191+
};
192+
193+
provider.HasStaticHandler(capabilities.TextDocument.Synchronization).Should().BeTrue();
194+
}
195+
196+
[Fact]
197+
public void GH162_TextDocumentSync_Should_Work_With_WillSave_Or_WillSaveWaitUntil()
198+
{
199+
var textDocumentSyncHandler = TextDocumentSyncHandlerExtensions.With(DocumentSelector.ForPattern("**/*.cs"), "csharp");
200+
var willSaveTextDocumentHandler = Substitute.For<IWillSaveTextDocumentHandler>();
201+
var willSaveWaitUntilTextDocumentHandler = Substitute.For<IWillSaveWaitUntilTextDocumentHandler>();
202+
var didSaveTextDocumentHandler = Substitute.For<IDidSaveTextDocumentHandler>();
203+
204+
var collection = new HandlerCollection(SupportedCapabilitiesFixture.AlwaysTrue, new TextDocumentIdentifiers()) { textDocumentSyncHandler, willSaveTextDocumentHandler, willSaveWaitUntilTextDocumentHandler, didSaveTextDocumentHandler };
205+
var provider = new ClientCapabilityProvider(collection);
206+
var capabilities = new ClientCapabilities()
207+
{
208+
TextDocument = new TextDocumentClientCapabilities()
209+
{
210+
Synchronization = new SynchronizationCapability()
211+
{
212+
DidSave = true,
213+
DynamicRegistration = false,
214+
WillSave = true,
215+
WillSaveWaitUntil = true
216+
},
217+
}
218+
};
219+
220+
provider.HasStaticHandler(capabilities.TextDocument.Synchronization).Should().BeTrue();
221+
}
222+
164223
private static bool HasHandler(ClientCapabilityProvider provider, object instance)
165224
{
166225
return (bool)typeof(ClientCapabilityProviderTests).GetTypeInfo()

‎test/Lsp.Tests/Models/InitializeResultTests.cs

+39-20
Original file line numberDiff line numberDiff line change
@@ -17,29 +17,36 @@ public class InitializeResultTests
1717
[Theory, JsonFixture]
1818
public void SimpleTest(string expected)
1919
{
20-
var model = new InitializeResult() {
21-
Capabilities = new ServerCapabilities() {
20+
var model = new InitializeResult()
21+
{
22+
Capabilities = new ServerCapabilities()
23+
{
2224
CodeActionProvider = true,
23-
CodeLensProvider = new CodeLensOptions() {
25+
CodeLensProvider = new CodeLensOptions()
26+
{
2427
ResolveProvider = true,
2528
},
26-
CompletionProvider = new CompletionOptions() {
29+
CompletionProvider = new CompletionOptions()
30+
{
2731
ResolveProvider = true,
2832
TriggerCharacters = new[] { "a", "b", "c" }
2933
},
3034
DefinitionProvider = true,
3135
DocumentFormattingProvider = true,
3236
DocumentHighlightProvider = true,
33-
DocumentLinkProvider = new DocumentLinkOptions() {
37+
DocumentLinkProvider = new DocumentLinkOptions()
38+
{
3439
ResolveProvider = true
3540
},
36-
DocumentOnTypeFormattingProvider = new DocumentOnTypeFormattingOptions() {
41+
DocumentOnTypeFormattingProvider = new DocumentOnTypeFormattingOptions()
42+
{
3743
FirstTriggerCharacter = ".",
3844
MoreTriggerCharacter = new[] { ";", " " }
3945
},
4046
DocumentRangeFormattingProvider = true,
4147
DocumentSymbolProvider = true,
42-
ExecuteCommandProvider = new ExecuteCommandOptions() {
48+
ExecuteCommandProvider = new ExecuteCommandOptions()
49+
{
4350
Commands = new string[] { "command1", "command2" }
4451
},
4552
Experimental = new Dictionary<string, JToken>() {
@@ -48,13 +55,16 @@ public void SimpleTest(string expected)
4855
HoverProvider = true,
4956
ReferencesProvider = true,
5057
RenameProvider = true,
51-
SignatureHelpProvider = new SignatureHelpOptions() {
58+
SignatureHelpProvider = new SignatureHelpOptions()
59+
{
5260
TriggerCharacters = new[] { ";", " " }
5361
},
54-
TextDocumentSync = new TextDocumentSync(new TextDocumentSyncOptions() {
62+
TextDocumentSync = new TextDocumentSync(new TextDocumentSyncOptions()
63+
{
5564
Change = TextDocumentSyncKind.Full,
5665
OpenClose = true,
57-
Save = new SaveOptions() {
66+
Save = new SaveOptions()
67+
{
5868
IncludeText = true
5969
},
6070
WillSave = true,
@@ -74,33 +84,42 @@ public void SimpleTest(string expected)
7484
[Theory, JsonFixture]
7585
public void BooleanOrTest(string expected)
7686
{
77-
var model = new InitializeResult() {
78-
Capabilities = new ServerCapabilities {
79-
CodeActionProvider = new CodeActionOptions {
80-
CodeActionKinds = new [] {
87+
var model = new InitializeResult()
88+
{
89+
Capabilities = new ServerCapabilities
90+
{
91+
CodeActionProvider = new CodeActionOptions
92+
{
93+
CodeActionKinds = new[] {
8194
CodeActionKind.QuickFix
8295
}
8396
},
84-
ColorProvider = new ColorOptions {
97+
ColorProvider = new ColorOptions
98+
{
8599
DocumentSelector = DocumentSelector.ForPattern("**/*.foo"),
86100
Id = "foo"
87101
},
88-
DeclarationProvider = new DeclarationOptions {
102+
DeclarationProvider = new DeclarationOptions
103+
{
89104
DocumentSelector = DocumentSelector.ForPattern("**/*.foo"),
90105
Id = "foo"
91106
},
92-
FoldingRangeProvider = new FoldingRangeOptions {
107+
FoldingRangeProvider = new FoldingRangeOptions
108+
{
93109
DocumentSelector = DocumentSelector.ForPattern("**/*.foo"),
94110
Id = "foo"
95111
},
96-
ImplementationProvider = new ImplementationOptions {
112+
ImplementationProvider = new ImplementationOptions
113+
{
97114
DocumentSelector = DocumentSelector.ForPattern("**/*.foo"),
98115
Id = "foo"
99116
},
100-
RenameProvider = new RenameOptions {
117+
RenameProvider = new RenameOptions
118+
{
101119
PrepareProvider = true
102120
},
103-
TypeDefinitionProvider = new TypeDefinitionOptions {
121+
TypeDefinitionProvider = new TypeDefinitionOptions
122+
{
104123
DocumentSelector = DocumentSelector.ForPattern("**/*.foo"),
105124
Id = "foo"
106125
}

0 commit comments

Comments
 (0)