Skip to content

Commit 7d6d8a6

Browse files
CyrusNajmabadiCyrusNajmabadi
authored
Fix typing of 'else' causing unwanted indentation of following statement (#81393)
2 parents 16324bb + de6dde6 commit 7d6d8a6

File tree

2 files changed

+125
-31
lines changed

2 files changed

+125
-31
lines changed

‎src/EditorFeatures/Test2/Formatting/FormattingCommandHandlerTests.vb‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,74 @@ class TestClass
7979
End Using
8080
End Sub
8181

82+
<WpfFact>
83+
Public Sub TypingElseKeywordDoesNotIndentFollowingNonBlockConstruct()
84+
Using state = TestStateFactory.CreateCSharpTestState(
85+
<Document>using System;
86+
class TestClass
87+
{
88+
void TestMethod(string[] args)
89+
{
90+
if (v != null)
91+
Console.WriteLine("v is not null");
92+
els$$
93+
94+
if (v != null)
95+
Console.WriteLine("v is not null");
96+
}
97+
}</Document>, includeFormatCommandHandler:=True)
98+
state.SendTypeChars("e")
99+
100+
AssertEx.Equal("using System;
101+
class TestClass
102+
{
103+
void TestMethod(string[] args)
104+
{
105+
if (v != null)
106+
Console.WriteLine(""v is not null"");
107+
else
108+
109+
if (v != null)
110+
Console.WriteLine(""v is not null"");
111+
}
112+
}", state.GetDocumentText())
113+
End Using
114+
End Sub
115+
116+
<WpfFact>
117+
Public Sub TypingElseKeywordDoesNotIndentFollowingBlockConstruct()
118+
Using state = TestStateFactory.CreateCSharpTestState(
119+
<Document>using System;
120+
class TestClass
121+
{
122+
void TestMethod(string[] args)
123+
{
124+
if (v != null)
125+
Console.WriteLine("v is not null");
126+
els$$
127+
{
128+
Console.WriteLine("v is null");
129+
}
130+
}
131+
}</Document>, includeFormatCommandHandler:=True)
132+
state.SendTypeChars("e")
133+
134+
AssertEx.Equal("using System;
135+
class TestClass
136+
{
137+
void TestMethod(string[] args)
138+
{
139+
if (v != null)
140+
Console.WriteLine(""v is not null"");
141+
else
142+
{
143+
Console.WriteLine(""v is null"");
144+
}
145+
}
146+
}", state.GetDocumentText())
147+
End Using
148+
End Sub
149+
82150
Private Shared Sub AssertVirtualCaretColumn(state As TestState, expectedCol As Integer)
83151
Dim caretLine = state.GetLineFromCurrentCaretPosition()
84152
Dim caret = state.GetCaretPoint()

‎src/Workspaces/SharedUtilitiesAndExtensions/Workspace/CSharp/Formatting/CSharpSyntaxFormattingService.cs‎

Lines changed: 57 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -79,37 +79,7 @@ public ImmutableArray<TextChange> GetFormattingChangesOnTypedCharacter(
7979
var token = root.FindToken(Math.Max(0, caretPosition - 1), findInsideTrivia: true);
8080
var formattingRules = GetFormattingRules(document, caretPosition, token);
8181

82-
// Do not attempt to format on open/close brace if autoformat on close brace feature is off, instead just smart
83-
// indent.
84-
//
85-
// We want this behavior because it's totally reasonable for a user to want to not have on automatic formatting
86-
// because they feel it is too aggressive. However, by default, if you have smart-indentation on and are just
87-
// hitting enter, you'll common have the caret placed one indent higher than your current construct. For
88-
// example, if you have:
89-
//
90-
// if (true)
91-
// $ <-- smart indent will have placed the caret here here.
92-
//
93-
// This is acceptable given that the user may want to just write a simple statement there. However, if they
94-
// start writing `{`, then things should snap over to be:
95-
//
96-
// if (true)
97-
// {
98-
//
99-
// Importantly, this is just an indentation change, no actual 'formatting' is done. We do the same with close
100-
// brace. If you have:
101-
//
102-
// if (...)
103-
// {
104-
// bad . ly ( for (mmated+code) ) ;
105-
// $ <-- smart indent will have placed the care here.
106-
//
107-
// If the user hits `}` then we will properly smart indent the `}` to match the `{`. However, we won't touch any
108-
// of the other code in that block, unlike if we were formatting.
109-
var onlySmartIndent =
110-
(token.IsKind(SyntaxKind.CloseBraceToken) && OnlySmartIndentCloseBrace(indentationOptions.AutoFormattingOptions)) ||
111-
(token.IsKind(SyntaxKind.OpenBraceToken) && OnlySmartIndentOpenBrace(indentationOptions.AutoFormattingOptions));
112-
82+
var onlySmartIndent = OnlySmartIndentBraceToken() || OnlySmartIndentElseKeyword();
11383
if (onlySmartIndent)
11484
{
11585
// if we're only doing smart indent, then ignore all edits to this token that occur before the span of the
@@ -124,6 +94,62 @@ public ImmutableArray<TextChange> GetFormattingChangesOnTypedCharacter(
12494
return changes;
12595

12696
return [.. FormatToken(document, indentationOptions, token, formattingRules, cancellationToken)];
97+
98+
bool OnlySmartIndentBraceToken()
99+
{
100+
// Do not attempt to format on open/close brace if autoformat on close brace feature is off, instead just smart
101+
// indent.
102+
//
103+
// We want this behavior because it's totally reasonable for a user to want to not have on automatic formatting
104+
// because they feel it is too aggressive. However, by default, if you have smart-indentation on and are just
105+
// hitting enter, you'll common have the caret placed one indent higher than your current construct. For
106+
// example, if you have:
107+
//
108+
// if (true)
109+
// $ <-- smart indent will have placed the caret here here.
110+
//
111+
// This is acceptable given that the user may want to just write a simple statement there. However, if they
112+
// start writing `{`, then things should snap over to be:
113+
//
114+
// if (true)
115+
// {
116+
//
117+
// Importantly, this is just an indentation change, no actual 'formatting' is done. We do the same with close
118+
// brace. If you have:
119+
//
120+
// if (...)
121+
// {
122+
// bad . ly ( for (mmated+code) ) ;
123+
// $ <-- smart indent will have placed the care here.
124+
//
125+
// If the user hits `}` then we will properly smart indent the `}` to match the `{`. However, we won't touch any
126+
// of the other code in that block, unlike if we were formatting.
127+
return
128+
(token.IsKind(SyntaxKind.CloseBraceToken) && OnlySmartIndentCloseBrace(indentationOptions.AutoFormattingOptions)) ||
129+
(token.IsKind(SyntaxKind.OpenBraceToken) && OnlySmartIndentOpenBrace(indentationOptions.AutoFormattingOptions));
130+
}
131+
132+
bool OnlySmartIndentElseKeyword()
133+
{
134+
// If the user has typed the `e` in `else` we only want to indent the 'else' token itself (the purpose is to
135+
// place the `else` token properly in cases like:
136+
//
137+
// foreach (var item in collection)
138+
// if (item.IsValid)
139+
// Process(item);
140+
// els //<-- user types 'e' here
141+
//
142+
// In this case we want to indent 'else' to match the 'if' above. We do not want to adjust the formatting of
143+
// anything else. This is especially important as 'else' takes an embedded statement, and we don't want the
144+
// following unrelated statement to get reformatted (unless it's a block that should travel with the else.
145+
if (token.Kind() != SyntaxKind.ElseKeyword)
146+
return false;
147+
148+
if (token.Parent is not ElseClauseSyntax elseClause)
149+
return false;
150+
151+
return elseClause.Statement is not BlockSyntax;
152+
}
127153
}
128154

129155
private static bool OnlySmartIndentCloseBrace(in AutoFormattingOptions options)

0 commit comments

Comments
 (0)