Skip to content

Fix typing of 'else' causing unwanted indentation of following statement #81393

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 3 commits into from
Nov 21, 2025
Merged

Fix typing of 'else' causing unwanted indentation of following statement #81393

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
24c42ec
Fix typing of 'else' causing unwanted indentation of following statement
CyrusNajmabadi Nov 21, 2025
ba19b2f
Apply suggestions from code review
CyrusNajmabadi Nov 21, 2025
de6dde6
pply suggestions from code review
JoeRobich Nov 21, 2025
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
68 changes: 68 additions & 0 deletions src/EditorFeatures/Test2/Formatting/FormattingCommandHandlerTests.vb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,74 @@ class TestClass
End Using
End Sub

<WpfFact>
Public Sub TypingElseKeywordDoesNotIndentFollowingNonBlockConstruct()
Using state = TestStateFactory.CreateCSharpTestState(
<Document>using System;
class TestClass
{
void TestMethod(string[] args)
{
if (v != null)
Console.WriteLine("v is not null");
els$$

if (v != null)
Console.WriteLine("v is not null");
}
}</Document>, includeFormatCommandHandler:=True)
state.SendTypeChars("e")

AssertEx.Equal("using System;
class TestClass
{
void TestMethod(string[] args)
{
if (v != null)
Console.WriteLine(""v is not null"");
else

if (v != null)
Console.WriteLine(""v is not null"");
}
}", state.GetDocumentText())
End Using
End Sub

<WpfFact>
Public Sub TypingElseKeywordDoesNotIndentFollowingBlockConstruct()
Using state = TestStateFactory.CreateCSharpTestState(
<Document>using System;
class TestClass
{
void TestMethod(string[] args)
{
if (v != null)
Console.WriteLine("v is not null");
els$$
{
Console.WriteLine("v is null");
}
}
}</Document>, includeFormatCommandHandler:=True)
state.SendTypeChars("e")

AssertEx.Equal("using System;
class TestClass
{
void TestMethod(string[] args)
{
if (v != null)
Console.WriteLine(""v is not null"");
else
{
Console.WriteLine(""v is null"");
}
}
}", state.GetDocumentText())
End Using
End Sub

Private Shared Sub AssertVirtualCaretColumn(state As TestState, expectedCol As Integer)
Dim caretLine = state.GetLineFromCurrentCaretPosition()
Dim caret = state.GetCaretPoint()
Expand Down
88 changes: 57 additions & 31 deletions ...SharedUtilitiesAndExtensions/Workspace/CSharp/Formatting/CSharpSyntaxFormattingService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,37 +79,7 @@ public ImmutableArray<TextChange> GetFormattingChangesOnTypedCharacter(
var token = root.FindToken(Math.Max(0, caretPosition - 1), findInsideTrivia: true);
var formattingRules = GetFormattingRules(document, caretPosition, token);

// Do not attempt to format on open/close brace if autoformat on close brace feature is off, instead just smart
// indent.
//
// We want this behavior because it's totally reasonable for a user to want to not have on automatic formatting
// because they feel it is too aggressive. However, by default, if you have smart-indentation on and are just
// hitting enter, you'll common have the caret placed one indent higher than your current construct. For
// example, if you have:
//
// if (true)
// $ <-- smart indent will have placed the caret here here.
//
// This is acceptable given that the user may want to just write a simple statement there. However, if they
// start writing `{`, then things should snap over to be:
//
// if (true)
// {
//
// Importantly, this is just an indentation change, no actual 'formatting' is done. We do the same with close
// brace. If you have:
//
// if (...)
// {
// bad . ly ( for (mmated+code) ) ;
// $ <-- smart indent will have placed the care here.
//
// If the user hits `}` then we will properly smart indent the `}` to match the `{`. However, we won't touch any
// of the other code in that block, unlike if we were formatting.
var onlySmartIndent =
(token.IsKind(SyntaxKind.CloseBraceToken) && OnlySmartIndentCloseBrace(indentationOptions.AutoFormattingOptions)) ||
(token.IsKind(SyntaxKind.OpenBraceToken) && OnlySmartIndentOpenBrace(indentationOptions.AutoFormattingOptions));

var onlySmartIndent = OnlySmartIndentBraceToken() || OnlySmartIndentElseKeyword();
if (onlySmartIndent)
{
// if we're only doing smart indent, then ignore all edits to this token that occur before the span of the
Expand All @@ -124,6 +94,62 @@ public ImmutableArray<TextChange> GetFormattingChangesOnTypedCharacter(
return changes;

return [.. FormatToken(document, indentationOptions, token, formattingRules, cancellationToken)];

bool OnlySmartIndentBraceToken()
{
// Do not attempt to format on open/close brace if autoformat on close brace feature is off, instead just smart
// indent.
//
// We want this behavior because it's totally reasonable for a user to want to not have on automatic formatting
// because they feel it is too aggressive. However, by default, if you have smart-indentation on and are just
// hitting enter, you'll common have the caret placed one indent higher than your current construct. For
// example, if you have:
//
// if (true)
// $ <-- smart indent will have placed the caret here here.
//
// This is acceptable given that the user may want to just write a simple statement there. However, if they
// start writing `{`, then things should snap over to be:
//
// if (true)
// {
//
// Importantly, this is just an indentation change, no actual 'formatting' is done. We do the same with close
// brace. If you have:
//
// if (...)
// {
// bad . ly ( for (mmated+code) ) ;
// $ <-- smart indent will have placed the care here.
//
// If the user hits `}` then we will properly smart indent the `}` to match the `{`. However, we won't touch any
// of the other code in that block, unlike if we were formatting.
return
(token.IsKind(SyntaxKind.CloseBraceToken) && OnlySmartIndentCloseBrace(indentationOptions.AutoFormattingOptions)) ||
(token.IsKind(SyntaxKind.OpenBraceToken) && OnlySmartIndentOpenBrace(indentationOptions.AutoFormattingOptions));
}

bool OnlySmartIndentElseKeyword()
{
// If the user has typed the `e` in `else` we only want to indent the 'else' token itself (the purpose is to
// place the `else` token properly in cases like:
//
// foreach (var item in collection)
// if (item.IsValid)
// Process(item);
// els //<-- user types 'e' here
//
// In this case we want to indent 'else' to match the 'if' above. We do not want to adjust the formatting of
// anything else. This is especially important as 'else' takes an embedded statement, and we don't want the
// following unrelated statement to get reformatted (unless it's a block that should travel with the else.
if (token.Kind() != SyntaxKind.ElseKeyword)
return false;

if (token.Parent is not ElseClauseSyntax elseClause)
return false;

return elseClause.Statement is not BlockSyntax;
}
}

private static bool OnlySmartIndentCloseBrace(in AutoFormattingOptions options)
Expand Down
Loading