Skip to content

Commit a6c772c

Browse files
Fix string splitter multi-caret handling (#80883)
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: CyrusNajmabadi <4564579+CyrusNajmabadi@users.noreply.github.com> Co-authored-by: Cyrus Najmabadi <cyrus.najmabadi@gmail.com>
1 parent e078eb7 commit a6c772c

File tree

2 files changed

+72
-24
lines changed

2 files changed

+72
-24
lines changed

src/EditorFeatures/CSharp/SplitStringLiteral/SplitStringLiteralCommandHandler.cs

Lines changed: 33 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
using System.ComponentModel.Composition;
77
using System.Linq;
88
using System.Threading;
9+
using Microsoft.CodeAnalysis.Collections;
910
using Microsoft.CodeAnalysis.CSharp.SplitStringLiteral;
1011
using Microsoft.CodeAnalysis.Editor.Shared.Extensions;
1112
using Microsoft.CodeAnalysis.Editor.Shared.Utilities;
@@ -86,6 +87,13 @@ public bool ExecuteCommandWorker(ReturnKeyCommandArgs args, CancellationToken ca
8687
// The list of spans is traversed in reverse order so we do not have to
8788
// deal with updating later caret positions to account for the added space
8889
// from splitting at earlier caret positions.
90+
//
91+
// However, we need to track the new caret positions in the original forward order
92+
// for setting the multi-selection at the end. We use tracking points to automatically
93+
// adjust positions as the buffer changes.
94+
using var _ = PooledObjects.ArrayBuilder<ITrackingPoint>.GetInstance(spans.Count, fillWithValue: null!, out var trackingPoints);
95+
96+
var currentIndex = spans.Count - 1;
8997
foreach (var span in spans.Reverse())
9098
{
9199
using var transaction = CaretPreservingEditTransaction.TryCreate(
@@ -103,23 +111,35 @@ public bool ExecuteCommandWorker(ReturnKeyCommandArgs args, CancellationToken ca
103111
var newSnapshot = subjectBuffer.ApplyChanges(newDocument.GetChanges(parsedDocument));
104112
parsedDocument = newDocument;
105113

106-
// The buffer edit may have adjusted to position of the current caret but we might need a different location.
107-
// Only adjust caret if it is the only one (no multi-caret support: https://github.com/dotnet/roslyn/issues/64812).
108-
if (spans.Count == 1)
109-
{
110-
var newCaretPoint = textView.BufferGraph.MapUpToBuffer(
111-
new SnapshotPoint(newSnapshot, newPosition),
112-
PointTrackingMode.Negative,
113-
PositionAffinity.Predecessor,
114-
textView.TextBuffer);
115-
116-
if (newCaretPoint != null)
117-
textView.Caret.MoveTo(newCaretPoint.Value);
118-
}
114+
// Create a tracking point for the new caret position.
115+
// Use Negative tracking mode so the caret stays before any text inserted at this position.
116+
trackingPoints[currentIndex] = newSnapshot.CreateTrackingPoint(newPosition, PointTrackingMode.Negative);
119117

120118
transaction?.Complete();
119+
currentIndex--;
120+
}
121+
122+
// Now set all the caret positions using the multi-selection broker.
123+
// Get the position of each tracking point in the final snapshot.
124+
var finalSnapshot = subjectBuffer.CurrentSnapshot;
125+
using var finalCaretSpans = TemporaryArray<SnapshotSpan>.Empty;
126+
127+
foreach (var trackingPoint in trackingPoints)
128+
{
129+
var position = trackingPoint.GetPosition(finalSnapshot);
130+
var newCaretPoint = textView.BufferGraph.MapUpToBuffer(
131+
new SnapshotPoint(finalSnapshot, position),
132+
PointTrackingMode.Negative,
133+
PositionAffinity.Predecessor,
134+
textView.TextBuffer);
135+
136+
if (newCaretPoint != null)
137+
finalCaretSpans.AsRef().Add(new SnapshotSpan(newCaretPoint.Value, 0));
121138
}
122139

140+
if (finalCaretSpans.Count > 0)
141+
textView.SetMultiSelection(finalCaretSpans.ToImmutableAndClear());
142+
123143
return true;
124144

125145
static bool LineContainsQuote(ITextSnapshotLine line, int caretPosition)

src/EditorFeatures/CSharpTest/SplitStringLiteral/SplitStringLiteralCommandHandlerTests.cs

Lines changed: 39 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
// See the LICENSE file in the project root for more information.
44

55
using System;
6+
using System.Diagnostics.CodeAnalysis;
67
using System.Linq;
78
using Microsoft.CodeAnalysis.Editor.CSharp.SplitStringLiteral;
89
using Microsoft.CodeAnalysis.Editor.Shared.Extensions;
@@ -27,10 +28,9 @@ namespace Microsoft.CodeAnalysis.Editor.CSharp.UnitTests.SplitStringLiteral;
2728
public sealed class SplitStringLiteralCommandHandlerTests
2829
{
2930
/// <summary>
30-
/// verifyUndo is needed because of https://github.com/dotnet/roslyn/issues/28033
31-
/// Most tests will continue to verifyUndo, but select tests will skip it due to
32-
/// this known test infrastructure issure. This bug does not represent a product
33-
/// failure.
31+
/// verifyUndo is needed because of https://github.com/dotnet/roslyn/issues/28033 Most tests will continue to
32+
/// verifyUndo, but select tests will skip it due to this known test infrastructure issue. This bug does not
33+
/// represent a product failure.
3434
/// </summary>
3535
private static void TestWorker(
3636
string inputMarkup,
@@ -113,14 +113,13 @@ private static void TestWorker(
113113
}
114114

115115
/// <summary>
116-
/// verifyUndo is needed because of https://github.com/dotnet/roslyn/issues/28033
117-
/// Most tests will continue to verifyUndo, but select tests will skip it due to
118-
/// this known test infrastructure issure. This bug does not represent a product
119-
/// failure.
116+
/// verifyUndo is needed because of https://github.com/dotnet/roslyn/issues/28033 Most tests will continue to
117+
/// verifyUndo, but select tests will skip it due to this known test infrastructure issue. This bug does not
118+
/// represent a product failure.
120119
/// </summary>
121120
private static void TestHandled(
122-
string inputMarkup,
123-
string expectedOutputMarkup,
121+
[StringSyntax(PredefinedEmbeddedLanguageNames.CSharpTest)] string inputMarkup,
122+
[StringSyntax(PredefinedEmbeddedLanguageNames.CSharpTest)] string expectedOutputMarkup,
124123
bool verifyUndo = true,
125124
IndentStyle indentStyle = IndentStyle.Smart,
126125
bool useTabs = false,
@@ -135,7 +134,8 @@ private static void TestHandled(
135134
verifyUndo, indentStyle, useTabs, endOfLine);
136135
}
137136

138-
private static void TestNotHandled(string inputMarkup)
137+
private static void TestNotHandled(
138+
[StringSyntax(PredefinedEmbeddedLanguageNames.CSharpTest)] string inputMarkup)
139139
{
140140
var notHandled = false;
141141
TestWorker(
@@ -1069,6 +1069,34 @@ void M()
10691069
}
10701070
""");
10711071

1072+
[WpfFact, WorkItem("https://github.com/dotnet/roslyn/issues/64812")]
1073+
public void TestMultiCaretPositionsAreAllSet()
1074+
{
1075+
var inputMarkup = """
1076+
class C
1077+
{
1078+
void M()
1079+
{
1080+
var v = "now is [||]the ti[||]me";
1081+
}
1082+
}
1083+
""";
1084+
1085+
var expectedMarkup = """
1086+
class C
1087+
{
1088+
void M()
1089+
{
1090+
var v = "now is " +
1091+
"[||]the ti" +
1092+
"[||]me";
1093+
}
1094+
}
1095+
""";
1096+
1097+
TestHandled(inputMarkup, expectedMarkup);
1098+
}
1099+
10721100
[WpfFact, WorkItem("https://github.com/dotnet/roslyn/issues/40277")]
10731101
public void TestInStringWithKeepTabsEnabled1()
10741102
=> TestHandled(

0 commit comments

Comments
 (0)