Skip to content

Commit ca3e79c

Browse files
Address code review feedback
- Use PooledObjects.ArrayBuilder for tracking points - Rename pooledSpans to finalCaretSpans - Simplify test to use TestHandled helper with markup Co-authored-by: CyrusNajmabadi <4564579+CyrusNajmabadi@users.noreply.github.com>
1 parent a0433d0 commit ca3e79c

File tree

2 files changed

+7
-49
lines changed

2 files changed

+7
-49
lines changed

src/EditorFeatures/CSharp/SplitStringLiteral/SplitStringLiteralCommandHandler.cs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ public bool ExecuteCommandWorker(ReturnKeyCommandArgs args, CancellationToken ca
9393
// However, we need to track the new caret positions in the original forward order
9494
// for setting the multi-selection at the end. We use tracking points to automatically
9595
// adjust positions as the buffer changes.
96-
var trackingPoints = new ITrackingPoint[spans.Count];
96+
using var _ = PooledObjects.ArrayBuilder<ITrackingPoint>.GetInstance(spans.Count, out var trackingPoints);
9797

9898
var currentIndex = spans.Count - 1;
9999
foreach (var span in spans.Reverse())
@@ -124,7 +124,7 @@ public bool ExecuteCommandWorker(ReturnKeyCommandArgs args, CancellationToken ca
124124
// Now set all the caret positions using the multi-selection broker.
125125
// Get the position of each tracking point in the final snapshot.
126126
var finalSnapshot = subjectBuffer.CurrentSnapshot;
127-
using var pooledSpans = TemporaryArray<SnapshotSpan>.Empty;
127+
using var finalCaretSpans = TemporaryArray<SnapshotSpan>.Empty;
128128

129129
foreach (var trackingPoint in trackingPoints)
130130
{
@@ -137,13 +137,13 @@ public bool ExecuteCommandWorker(ReturnKeyCommandArgs args, CancellationToken ca
137137

138138
if (newCaretPoint != null)
139139
{
140-
pooledSpans.AsRef().Add(new SnapshotSpan(newCaretPoint.Value, 0));
140+
finalCaretSpans.AsRef().Add(new SnapshotSpan(newCaretPoint.Value, 0));
141141
}
142142
}
143143

144-
if (pooledSpans.Count > 0)
144+
if (finalCaretSpans.Count > 0)
145145
{
146-
textView.SetMultiSelection(pooledSpans.ToImmutableAndClear());
146+
textView.SetMultiSelection(finalCaretSpans.ToImmutableAndClear());
147147
}
148148

149149
return true;

src/EditorFeatures/CSharpTest/SplitStringLiteral/SplitStringLiteralCommandHandlerTests.cs

Lines changed: 2 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -1072,39 +1072,16 @@ void M()
10721072
[WpfFact, WorkItem("https://github.com/dotnet/roslyn/issues/64812")]
10731073
public void TestMultiCaretPositionsAreAllSet()
10741074
{
1075-
var workspaceXml = """
1076-
<Workspace>
1077-
<Project Language="C#">
1078-
<Document>
1075+
var inputMarkup = """
10791076
class C
10801077
{
10811078
void M()
10821079
{
10831080
var v = "now is [||]the ti[||]me";
10841081
}
10851082
}
1086-
</Document>
1087-
</Project>
1088-
</Workspace>
10891083
""";
10901084

1091-
using var workspace = EditorTestWorkspace.Create(workspaceXml);
1092-
var document = workspace.Documents.Single();
1093-
var view = document.GetTextView();
1094-
var textBuffer = view.TextBuffer;
1095-
1096-
var originalSnapshot = textBuffer.CurrentSnapshot;
1097-
var originalSelections = document.SelectedSpans;
1098-
1099-
// Set up multi-selection with primary caret at the last position
1100-
view.SetMultiSelection(originalSelections.Select(selection => selection.ToSnapshotSpan(originalSnapshot)));
1101-
1102-
var commandHandler = workspace.ExportProvider.GetCommandHandler<SplitStringLiteralCommandHandler>(nameof(SplitStringLiteralCommandHandler));
1103-
var result = commandHandler.ExecuteCommand(new ReturnKeyCommandArgs(view, textBuffer), TestCommandExecutionContext.Create());
1104-
1105-
Assert.True(result);
1106-
1107-
// Verify the expected markup with all caret positions
11081085
var expectedMarkup = """
11091086
class C
11101087
{
@@ -1117,26 +1094,7 @@ void M()
11171094
}
11181095
""";
11191096

1120-
MarkupTestFile.GetSpans(expectedMarkup, out var expectedOutput, out var expectedSpans);
1121-
var actualText = textBuffer.CurrentSnapshot.AsText().ToString();
1122-
1123-
// Verify text is correct
1124-
Assert.Equal(expectedOutput, actualText);
1125-
1126-
// Verify all caret positions are set correctly
1127-
var broker = view.GetMultiSelectionBroker();
1128-
var selections = broker.AllSelections.ToList();
1129-
Assert.Equal(expectedSpans.Length, selections.Count);
1130-
1131-
// Verify each selection position matches the expected position
1132-
for (int i = 0; i < expectedSpans.Length; i++)
1133-
{
1134-
Assert.Equal(expectedSpans[i].Start, selections[i].Start.Position.Position);
1135-
Assert.True(selections[i].IsEmpty); // All selections should be carets (zero-length)
1136-
}
1137-
1138-
// Verify the primary (last) caret is at the correct position
1139-
Assert.Equal(expectedSpans.Last().Start, view.Caret.Position.BufferPosition.Position);
1097+
TestHandled(inputMarkup, expectedMarkup);
11401098
}
11411099

11421100
[WpfFact, WorkItem("https://github.com/dotnet/roslyn/issues/40277")]

0 commit comments

Comments
 (0)