From c9185a0af0779957466b6720ecb4ac0fd6a7428f Mon Sep 17 00:00:00 2001 From: Joey Robichaud Date: Wed, 22 Jan 2025 16:32:32 -0800 Subject: [PATCH 1/3] Update formatOnType handler to support formatting on NewLine --- .../Formatting/FormatDocumentOnTypeHandler.cs | 13 +- .../Formatting/FormatDocumentOnTypeTests.cs | 127 ++++++++++++------ 2 files changed, 94 insertions(+), 46 deletions(-) diff --git a/src/LanguageServer/Protocol/Handler/Formatting/FormatDocumentOnTypeHandler.cs b/src/LanguageServer/Protocol/Handler/Formatting/FormatDocumentOnTypeHandler.cs index e6e6df6215855..fcc61833a3598 100644 --- a/src/LanguageServer/Protocol/Handler/Formatting/FormatDocumentOnTypeHandler.cs +++ b/src/LanguageServer/Protocol/Handler/Formatting/FormatDocumentOnTypeHandler.cs @@ -44,13 +44,13 @@ public FormatDocumentOnTypeHandler(IGlobalOptionService globalOptions) if (document is null) return null; - var position = await document.GetPositionFromLinePositionAsync(ProtocolConversions.PositionToLinePosition(request.Position), cancellationToken).ConfigureAwait(false); - - if (string.IsNullOrEmpty(request.Character) || SyntaxFacts.IsNewLine(request.Character[0])) + if (string.IsNullOrEmpty(request.Character)) { return []; } + var position = await document.GetPositionFromLinePositionAsync(ProtocolConversions.PositionToLinePosition(request.Position), cancellationToken).ConfigureAwait(false); + var formattingService = document.Project.Services.GetRequiredService(); var documentSyntax = await ParsedDocument.CreateAsync(document, cancellationToken).ConfigureAwait(false); @@ -72,6 +72,13 @@ public FormatDocumentOnTypeHandler(IGlobalOptionService globalOptions) return []; } + if (SyntaxFacts.IsNewLine(request.Character[0])) + { + // When formatting after a newline is pressed, the cursor line will be blank and we do + // not want to remove the whitespace indentation from it. + textChanges = textChanges.WhereAsArray(change => !change.Span.Contains(position)); + } + return [.. textChanges.Select(change => ProtocolConversions.TextChangeToTextEdit(change, documentSyntax.Text))]; } } diff --git a/src/LanguageServer/ProtocolUnitTests/Formatting/FormatDocumentOnTypeTests.cs b/src/LanguageServer/ProtocolUnitTests/Formatting/FormatDocumentOnTypeTests.cs index 1ee3fbc732ffe..90e569d619cdf 100644 --- a/src/LanguageServer/ProtocolUnitTests/Formatting/FormatDocumentOnTypeTests.cs +++ b/src/LanguageServer/ProtocolUnitTests/Formatting/FormatDocumentOnTypeTests.cs @@ -2,8 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. -#nullable disable - +using System.Diagnostics.CodeAnalysis; using System.Linq; using System.Threading; using System.Threading.Tasks; @@ -24,65 +23,107 @@ public FormatDocumentOnTypeTests(ITestOutputHelper testOutputHelper) : base(test public async Task TestFormatDocumentOnTypeAsync(bool mutatingLspWorkspace) { var markup = -@"class A -{ - void M() - { - if (true) - {{|type:|} - } -}"; + """ + class A + { + void M() + { + if (true) + {{|type:|} + } + } + """; var expected = -@"class A -{ - void M() - { - if (true) - { - } -}"; + """ + class A + { + void M() + { + if (true) + { + } + } + """; await using var testLspServer = await CreateTestLspServerAsync(markup, mutatingLspWorkspace); var characterTyped = ";"; var locationTyped = testLspServer.GetLocations("type").Single(); - var documentText = await testLspServer.GetDocumentTextAsync(locationTyped.Uri); - - var results = await RunFormatDocumentOnTypeAsync(testLspServer, characterTyped, locationTyped); - var actualText = ApplyTextEdits(results, documentText); - Assert.Equal(expected, actualText); + await AssertFormatDocumentOnTypeAsync(testLspServer, characterTyped, locationTyped, expected); } [Theory, CombinatorialData] public async Task TestFormatDocumentOnType_UseTabsAsync(bool mutatingLspWorkspace) { var markup = -@"class A -{ - void M() - { - if (true) - {{|type:|} - } -}"; + """ + class A + { + void M() + { + if (true) + {{|type:|} + } + } + """; var expected = -@"class A -{ - void M() - { - if (true) - { - } -}"; + """ + class A + { + void M() + { + if (true) + { + } + } + """; await using var testLspServer = await CreateTestLspServerAsync(markup, mutatingLspWorkspace); var characterTyped = ";"; var locationTyped = testLspServer.GetLocations("type").Single(); - var documentText = await testLspServer.GetDocumentTextAsync(locationTyped.Uri); + await AssertFormatDocumentOnTypeAsync(testLspServer, characterTyped, locationTyped, expected, insertSpaces: false, tabSize: 4); + } - var results = await RunFormatDocumentOnTypeAsync(testLspServer, characterTyped, locationTyped, insertSpaces: false, tabSize: 4); + [Theory, CombinatorialData] + public async Task TestFormatDocumentOnType_NewLine(bool mutatingLspWorkspace) + { + var markup = + """ + class A + { + void M() { + {|type:|} + } + } + """; + var expected = + """ + class A + { + void M() + { + + } + } + """; + await using var testLspServer = await CreateTestLspServerAsync(markup, mutatingLspWorkspace); + var characterTyped = "\n"; + var locationTyped = testLspServer.GetLocations("type").Single(); + await AssertFormatDocumentOnTypeAsync(testLspServer, characterTyped, locationTyped, expected); + } + + private static async Task AssertFormatDocumentOnTypeAsync( + TestLspServer testLspServer, + string characterTyped, + LSP.Location locationTyped, + [StringSyntax(PredefinedEmbeddedLanguageNames.CSharpTest)] string expectedText, + bool insertSpaces = true, + int tabSize = 4) + { + var documentText = await testLspServer.GetDocumentTextAsync(locationTyped.Uri); + var results = await RunFormatDocumentOnTypeAsync(testLspServer, characterTyped, locationTyped, insertSpaces, tabSize); var actualText = ApplyTextEdits(results, documentText); - Assert.Equal(expected, actualText); + Assert.Equal(expectedText, actualText); } - private static async Task RunFormatDocumentOnTypeAsync( + private static async Task RunFormatDocumentOnTypeAsync( TestLspServer testLspServer, string characterTyped, LSP.Location locationTyped, From 3e87bdc6103ddd6852be2bf9d29d06ddeac7e57e Mon Sep 17 00:00:00 2001 From: Joey Robichaud Date: Wed, 22 Jan 2025 17:25:01 -0800 Subject: [PATCH 2/3] Review feedback --- .../Formatting/FormatDocumentOnTypeHandler.cs | 29 +++++++++++++++++-- .../Formatting/FormatDocumentOnTypeTests.cs | 17 +++-------- 2 files changed, 30 insertions(+), 16 deletions(-) diff --git a/src/LanguageServer/Protocol/Handler/Formatting/FormatDocumentOnTypeHandler.cs b/src/LanguageServer/Protocol/Handler/Formatting/FormatDocumentOnTypeHandler.cs index fcc61833a3598..ea99a0a4c0e45 100644 --- a/src/LanguageServer/Protocol/Handler/Formatting/FormatDocumentOnTypeHandler.cs +++ b/src/LanguageServer/Protocol/Handler/Formatting/FormatDocumentOnTypeHandler.cs @@ -74,9 +74,32 @@ public FormatDocumentOnTypeHandler(IGlobalOptionService globalOptions) if (SyntaxFacts.IsNewLine(request.Character[0])) { - // When formatting after a newline is pressed, the cursor line will be blank and we do - // not want to remove the whitespace indentation from it. - textChanges = textChanges.WhereAsArray(change => !change.Span.Contains(position)); + // When formatting after a newline is pressed, the cursor line will be all whitespace + // and we do not want to remove the indentation from it. + // + // Take the following example of pressing enter after an opening brace. + // + // ``` + // public void M() {||} + // ``` + // + // The editor moves the cursor to the next line and uses it's languageconfig to add + // the appropriate level of indentation. + // + // ``` + // public void M() { + // || + // } + // ``` + // + // At this point `formatOnType` is called. The formatting service will generate two + // text changes. The first moves the opening brace to the following line with proper + // indentation. The second removes the whitespace from the cursor line. + // + // Letting the second change go through would be a bad experience for the user as they + // will now be responsible for adding back the proper indentation. + + textChanges = textChanges.WhereAsArray(static (change, position) => !change.Span.Contains(position), position); } return [.. textChanges.Select(change => ProtocolConversions.TextChangeToTextEdit(change, documentSyntax.Text))]; diff --git a/src/LanguageServer/ProtocolUnitTests/Formatting/FormatDocumentOnTypeTests.cs b/src/LanguageServer/ProtocolUnitTests/Formatting/FormatDocumentOnTypeTests.cs index 90e569d619cdf..4540faebc839f 100644 --- a/src/LanguageServer/ProtocolUnitTests/Formatting/FormatDocumentOnTypeTests.cs +++ b/src/LanguageServer/ProtocolUnitTests/Formatting/FormatDocumentOnTypeTests.cs @@ -118,23 +118,14 @@ private static async Task AssertFormatDocumentOnTypeAsync( int tabSize = 4) { var documentText = await testLspServer.GetDocumentTextAsync(locationTyped.Uri); - var results = await RunFormatDocumentOnTypeAsync(testLspServer, characterTyped, locationTyped, insertSpaces, tabSize); + var results = await testLspServer.ExecuteRequestAsync( + LSP.Methods.TextDocumentOnTypeFormattingName, + CreateDocumentOnTypeFormattingParams(characterTyped, locationTyped, insertSpaces, tabSize), + CancellationToken.None); var actualText = ApplyTextEdits(results, documentText); Assert.Equal(expectedText, actualText); } - private static async Task RunFormatDocumentOnTypeAsync( - TestLspServer testLspServer, - string characterTyped, - LSP.Location locationTyped, - bool insertSpaces = true, - int tabSize = 4) - { - return await testLspServer.ExecuteRequestAsync(LSP.Methods.TextDocumentOnTypeFormattingName, - CreateDocumentOnTypeFormattingParams( - characterTyped, locationTyped, insertSpaces, tabSize), CancellationToken.None); - } - private static LSP.DocumentOnTypeFormattingParams CreateDocumentOnTypeFormattingParams( string characterTyped, LSP.Location locationTyped, From 05dbe1c92d0f5bae4bfff6d44bbdb52bcc541845 Mon Sep 17 00:00:00 2001 From: Joey Robichaud Date: Wed, 22 Jan 2025 17:50:45 -0800 Subject: [PATCH 3/3] Update comment --- .../Formatting/FormatDocumentOnTypeHandler.cs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/LanguageServer/Protocol/Handler/Formatting/FormatDocumentOnTypeHandler.cs b/src/LanguageServer/Protocol/Handler/Formatting/FormatDocumentOnTypeHandler.cs index ea99a0a4c0e45..f2191c061bc9f 100644 --- a/src/LanguageServer/Protocol/Handler/Formatting/FormatDocumentOnTypeHandler.cs +++ b/src/LanguageServer/Protocol/Handler/Formatting/FormatDocumentOnTypeHandler.cs @@ -80,21 +80,22 @@ public FormatDocumentOnTypeHandler(IGlobalOptionService globalOptions) // Take the following example of pressing enter after an opening brace. // // ``` - // public void M() {||} + // public void M() {||} // ``` // // The editor moves the cursor to the next line and uses it's languageconfig to add // the appropriate level of indentation. // // ``` - // public void M() { - // || - // } + // public void M() { + // || + // } // ``` // // At this point `formatOnType` is called. The formatting service will generate two - // text changes. The first moves the opening brace to the following line with proper - // indentation. The second removes the whitespace from the cursor line. + // text changes. The first moves the opening brace to a new line with proper + // indentation. The second removes the whitespace from the cursor line and rewrites + // the indentation prior to the closing brace. // // Letting the second change go through would be a bad experience for the user as they // will now be responsible for adding back the proper indentation.