Skip to content

Changes for #162 correct how text document sync settings are configured #163

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Oct 12, 2019
Merged

Changes for #162 correct how text document sync settings are configured #163

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
e66783b
Changes for #162 correct how text document sync settings are configured
david-driscoll Aug 7, 2019
52a13f0
Slight change
david-driscoll Aug 7, 2019
b5ec461
simplify the settings
david-driscoll Aug 7, 2019
648581d
remove extra braces
david-driscoll Aug 7, 2019
a730b68
Merge remote-tracking branch 'origin/master' into fix/text-options-de…
david-driscoll Oct 12, 2019
155948f
ensure that one of the changes includes text document sync
david-driscoll Oct 12, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion src/Protocol/Client/Capabilities/SynchronizationCapability.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,13 @@

namespace OmniSharp.Extensions.LanguageServer.Protocol.Client.Capabilities
{
public class SynchronizationCapability : DynamicCapability, ConnectedCapability<IDidChangeTextDocumentHandler>, ConnectedCapability<IDidCloseTextDocumentHandler>, ConnectedCapability<IDidOpenTextDocumentHandler>, ConnectedCapability<IDidSaveTextDocumentHandler>, ConnectedCapability<IWillSaveTextDocumentHandler>, ConnectedCapability<IWillSaveWaitUntilTextDocumentHandler>
public class SynchronizationCapability : DynamicCapability,
ConnectedCapability<IDidChangeTextDocumentHandler>,
ConnectedCapability<IDidCloseTextDocumentHandler>,
ConnectedCapability<IDidOpenTextDocumentHandler>,
ConnectedCapability<IDidSaveTextDocumentHandler>,
ConnectedCapability<IWillSaveTextDocumentHandler>,
ConnectedCapability<IWillSaveWaitUntilTextDocumentHandler>
{
/// <summary>
/// The client supports sending will save notifications.
Expand Down
20 changes: 14 additions & 6 deletions src/Protocol/Server/Capabilities/TextDocumentSyncOptions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using System;
using System.Collections.Generic;
using System.Linq;
using Newtonsoft.Json;
Expand Down Expand Up @@ -38,17 +39,24 @@ public class TextDocumentSyncOptions : ITextDocumentSyncOptions

public static TextDocumentSyncOptions Of(IEnumerable<ITextDocumentSyncOptions> options)
{
return new TextDocumentSyncOptions() {
OpenClose = options.Any(z => z.OpenClose),
Change = options
var change = TextDocumentSyncKind.None;
if (options.Any(x => x.Change != TextDocumentSyncKind.None))
{
change = options
.Where(x => x.Change != TextDocumentSyncKind.None)
.Min(z => z.Change),
.Min(z => z.Change);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As per comment in #162 (comment) this seems to throw because sequence doesn't contain any elements. Maybe add the same predicate x => x.Change != TextDocumentSyncKind.None also to the if (options.Any()) check above?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sounds like a good change...

}
return new TextDocumentSyncOptions()
{
OpenClose = options.Any(z => z.OpenClose),
Change = change,
WillSave = options.Any(z => z.WillSave),
WillSaveWaitUntil = options.Any(z => z.WillSaveWaitUntil),
Save = new SaveOptions() {
Save = new SaveOptions()
{
IncludeText = options.Any(z => z.Save?.IncludeText == true)
}
};
};
}
}
}
2 changes: 1 addition & 1 deletion src/Server/ClientCapabilityProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ public bool HasStaticHandler<T>(Supports<T> capability)
.Where(x => x.GetTypeInfo().IsGenericType && x.GetTypeInfo().GetGenericTypeDefinition() == typeof(ConnectedCapability<>))
.Select(x => x.GetTypeInfo().GetGenericArguments()[0].GetTypeInfo());

return handlerTypes.All(_collection.ContainsHandler);
return handlerTypes.Any(_collection.ContainsHandler);
}

public IOptionsGetter GetStaticOptions<T>(Supports<T> capability)
Expand Down
22 changes: 15 additions & 7 deletions src/Server/LanguageServer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,8 @@ private IDisposable RegisterHandlers(LspHandlerDescriptorDisposable handlerDispo
{
var registrations = handlerDisposable.Descriptors
.Where(d => d.AllowsDynamicRegistration)
.Select(d => new Registration() {
.Select(d => new Registration()
{
Id = d.Id.ToString(),
Method = d.Method,
RegisterOptions = d.RegistrationOptions
Expand Down Expand Up @@ -433,13 +434,20 @@ async Task<InitializeResult> IRequestHandler<InitializeParams, InitializeResult>

if (ccp.HasStaticHandler(textDocumentCapabilities.Synchronization))
{
var textDocumentSyncKind = _collection.ContainsHandler(typeof(IDidChangeTextDocumentHandler))
? _collection
var textDocumentSyncKind = TextDocumentSyncKind.None;
if (_collection.ContainsHandler(typeof(IDidChangeTextDocumentHandler)))
{
var kinds = _collection
.Select(x => x.Handler)
.OfType<IDidChangeTextDocumentHandler>()
.Where(x => x.GetRegistrationOptions()?.SyncKind != TextDocumentSyncKind.None)
.Min(z => z.GetRegistrationOptions()?.SyncKind)
: TextDocumentSyncKind.None;
.Select(x => x.GetRegistrationOptions()?.SyncKind ?? TextDocumentSyncKind.None)
.Where(x => x != TextDocumentSyncKind.None)
.ToArray();
if (kinds.Any())
{
textDocumentSyncKind = kinds.Min(z => z);
}
}

if (_clientVersion == ClientVersion.Lsp2)
{
Expand All @@ -449,7 +457,7 @@ async Task<InitializeResult> IRequestHandler<InitializeParams, InitializeResult>
{
serverCapabilities.TextDocumentSync = new TextDocumentSyncOptions()
{
Change = textDocumentSyncKind ?? TextDocumentSyncKind.None,
Change = TextDocumentSyncKind.None,
OpenClose = _collection.ContainsHandler(typeof(IDidOpenTextDocumentHandler)) || _collection.ContainsHandler(typeof(IDidCloseTextDocumentHandler)),
Save = _collection.ContainsHandler(typeof(IDidSaveTextDocumentHandler)) ?
new SaveOptions() { IncludeText = true /* TODO: Make configurable */ } :
Expand Down
75 changes: 67 additions & 8 deletions test/Lsp.Tests/ClientCapabilityProviderTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@ public void Should_AllowSupportedCapabilities(IJsonRpcHandler handler, object in

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

public static IEnumerable<object[]> AllowUnsupportedCapabilities()
{
return GetItems(Capabilities, type => {
return GetItems(Capabilities, type =>
{
var handlerTypes = GetHandlerTypes(type);
var handler = Substitute.For(handlerTypes.ToArray(), new object[0]);
return new[] { handler, Activator.CreateInstance(typeof(Supports<>).MakeGenericType(type), false) };
Expand Down Expand Up @@ -104,7 +106,8 @@ public void Should_AllowNullSupportedCapabilities(IJsonRpcHandler handler, objec

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

public static IEnumerable<object[]> DisallowDynamicSupportsCapabilities()
{
return GetItems(Capabilities, type => {
return GetItems(Capabilities, type =>
{
var handlerTypes = GetHandlerTypes(type);
var handler = Substitute.For(handlerTypes.ToArray(), new object[0]);
var capability = Activator.CreateInstance(type);
Expand All @@ -145,12 +149,16 @@ public void Should_Handle_Mixed_Capabilities()

var collection = new HandlerCollection(SupportedCapabilitiesFixture.AlwaysTrue, new TextDocumentIdentifiers()) { textDocumentSyncHandler, codeActionHandler, definitionHandler, typeDefinitionHandler };
var provider = new ClientCapabilityProvider(collection);
var capabilities = new ClientCapabilities() {
TextDocument = new TextDocumentClientCapabilities() {
CodeAction = new Supports<CodeActionCapability>(true, new CodeActionCapability() {
var capabilities = new ClientCapabilities()
{
TextDocument = new TextDocumentClientCapabilities()
{
CodeAction = new Supports<CodeActionCapability>(true, new CodeActionCapability()
{
DynamicRegistration = false,
}),
TypeDefinition = new Supports<TypeDefinitionCapability>(true, new TypeDefinitionCapability() {
TypeDefinition = new Supports<TypeDefinitionCapability>(true, new TypeDefinitionCapability()
{
DynamicRegistration = true,
})
}
Expand All @@ -161,6 +169,57 @@ public void Should_Handle_Mixed_Capabilities()
provider.HasStaticHandler(capabilities.TextDocument.TypeDefinition).Should().BeFalse();
}

[Fact]
public void GH162_TextDocumentSync_Should_Work_Without_WillSave_Or_WillSaveWaitUntil()
{
var textDocumentSyncHandler = TextDocumentSyncHandlerExtensions.With(DocumentSelector.ForPattern("**/*.cs"), "csharp");

var collection = new HandlerCollection(SupportedCapabilitiesFixture.AlwaysTrue, new TextDocumentIdentifiers()) { textDocumentSyncHandler };
var provider = new ClientCapabilityProvider(collection);
var capabilities = new ClientCapabilities()
{
TextDocument = new TextDocumentClientCapabilities()
{
Synchronization = new SynchronizationCapability()
{
DidSave = true,
DynamicRegistration = false,
WillSave = true,
WillSaveWaitUntil = true
},
}
};

provider.HasStaticHandler(capabilities.TextDocument.Synchronization).Should().BeTrue();
}

[Fact]
public void GH162_TextDocumentSync_Should_Work_With_WillSave_Or_WillSaveWaitUntil()
{
var textDocumentSyncHandler = TextDocumentSyncHandlerExtensions.With(DocumentSelector.ForPattern("**/*.cs"), "csharp");
var willSaveTextDocumentHandler = Substitute.For<IWillSaveTextDocumentHandler>();
var willSaveWaitUntilTextDocumentHandler = Substitute.For<IWillSaveWaitUntilTextDocumentHandler>();
var didSaveTextDocumentHandler = Substitute.For<IDidSaveTextDocumentHandler>();

var collection = new HandlerCollection(SupportedCapabilitiesFixture.AlwaysTrue, new TextDocumentIdentifiers()) { textDocumentSyncHandler, willSaveTextDocumentHandler, willSaveWaitUntilTextDocumentHandler, didSaveTextDocumentHandler };
var provider = new ClientCapabilityProvider(collection);
var capabilities = new ClientCapabilities()
{
TextDocument = new TextDocumentClientCapabilities()
{
Synchronization = new SynchronizationCapability()
{
DidSave = true,
DynamicRegistration = false,
WillSave = true,
WillSaveWaitUntil = true
},
}
};

provider.HasStaticHandler(capabilities.TextDocument.Synchronization).Should().BeTrue();
}

private static bool HasHandler(ClientCapabilityProvider provider, object instance)
{
return (bool)typeof(ClientCapabilityProviderTests).GetTypeInfo()
Expand Down
59 changes: 39 additions & 20 deletions test/Lsp.Tests/Models/InitializeResultTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,29 +17,36 @@ public class InitializeResultTests
[Theory, JsonFixture]
public void SimpleTest(string expected)
{
var model = new InitializeResult() {
Capabilities = new ServerCapabilities() {
var model = new InitializeResult()
{
Capabilities = new ServerCapabilities()
{
CodeActionProvider = true,
CodeLensProvider = new CodeLensOptions() {
CodeLensProvider = new CodeLensOptions()
{
ResolveProvider = true,
},
CompletionProvider = new CompletionOptions() {
CompletionProvider = new CompletionOptions()
{
ResolveProvider = true,
TriggerCharacters = new[] { "a", "b", "c" }
},
DefinitionProvider = true,
DocumentFormattingProvider = true,
DocumentHighlightProvider = true,
DocumentLinkProvider = new DocumentLinkOptions() {
DocumentLinkProvider = new DocumentLinkOptions()
{
ResolveProvider = true
},
DocumentOnTypeFormattingProvider = new DocumentOnTypeFormattingOptions() {
DocumentOnTypeFormattingProvider = new DocumentOnTypeFormattingOptions()
{
FirstTriggerCharacter = ".",
MoreTriggerCharacter = new[] { ";", " " }
},
DocumentRangeFormattingProvider = true,
DocumentSymbolProvider = true,
ExecuteCommandProvider = new ExecuteCommandOptions() {
ExecuteCommandProvider = new ExecuteCommandOptions()
{
Commands = new string[] { "command1", "command2" }
},
Experimental = new Dictionary<string, JToken>() {
Expand All @@ -48,13 +55,16 @@ public void SimpleTest(string expected)
HoverProvider = true,
ReferencesProvider = true,
RenameProvider = true,
SignatureHelpProvider = new SignatureHelpOptions() {
SignatureHelpProvider = new SignatureHelpOptions()
{
TriggerCharacters = new[] { ";", " " }
},
TextDocumentSync = new TextDocumentSync(new TextDocumentSyncOptions() {
TextDocumentSync = new TextDocumentSync(new TextDocumentSyncOptions()
{
Change = TextDocumentSyncKind.Full,
OpenClose = true,
Save = new SaveOptions() {
Save = new SaveOptions()
{
IncludeText = true
},
WillSave = true,
Expand All @@ -74,33 +84,42 @@ public void SimpleTest(string expected)
[Theory, JsonFixture]
public void BooleanOrTest(string expected)
{
var model = new InitializeResult() {
Capabilities = new ServerCapabilities {
CodeActionProvider = new CodeActionOptions {
CodeActionKinds = new [] {
var model = new InitializeResult()
{
Capabilities = new ServerCapabilities
{
CodeActionProvider = new CodeActionOptions
{
CodeActionKinds = new[] {
CodeActionKind.QuickFix
}
},
ColorProvider = new ColorOptions {
ColorProvider = new ColorOptions
{
DocumentSelector = DocumentSelector.ForPattern("**/*.foo"),
Id = "foo"
},
DeclarationProvider = new DeclarationOptions {
DeclarationProvider = new DeclarationOptions
{
DocumentSelector = DocumentSelector.ForPattern("**/*.foo"),
Id = "foo"
},
FoldingRangeProvider = new FoldingRangeOptions {
FoldingRangeProvider = new FoldingRangeOptions
{
DocumentSelector = DocumentSelector.ForPattern("**/*.foo"),
Id = "foo"
},
ImplementationProvider = new ImplementationOptions {
ImplementationProvider = new ImplementationOptions
{
DocumentSelector = DocumentSelector.ForPattern("**/*.foo"),
Id = "foo"
},
RenameProvider = new RenameOptions {
RenameProvider = new RenameOptions
{
PrepareProvider = true
},
TypeDefinitionProvider = new TypeDefinitionOptions {
TypeDefinitionProvider = new TypeDefinitionOptions
{
DocumentSelector = DocumentSelector.ForPattern("**/*.foo"),
Id = "foo"
}
Expand Down