Fix string splitter multi-caret handling by Copilot · Pull Request #80883 · dotnet/roslyn

Copilot · 2025-10-24T14:06:46Z

Fix string splitter multi-caret handling

Plan

Explore codebase and understand the issue
Implement multi-caret position tracking during string splits
Use IMultiSelectionBroker to set all caret positions after edits
Use tracking points to handle position changes across edits
Add tests for multi-caret scenarios
Code review completed - addressed all feedback
Security scan completed - no vulnerabilities detected

Summary

Fixed issue #64812 where the string splitter incorrectly handles multi-caret scenarios.

Problem:
When splitting strings with multiple carets:

Only the single-caret case was supported
Multi-caret scenarios resulted in incorrect caret positions
Primary caret could end up in the wrong location
Additional unwanted carets were sometimes added

Solution:

Track new caret positions using ITrackingPoint as each split is processed (in reverse order)
Tracking points automatically adjust positions as subsequent edits are applied
After all edits are complete, get the final position of each tracking point
Use SetMultiSelection extension method to set all carets at once

Changes Made

Modified SplitStringLiteralCommandHandler.cs:

Use PooledObjects.ArrayBuilder for tracking point collection
Renamed pooledSpans to finalCaretSpans for clarity
After all splits complete, resolve tracking points to final positions
Use textView.SetMultiSelection() to set all carets at once (with the last as the primary)

Simplified test TestMultiCaretPositionsAreAllSet:

Now uses TestHandled helper method with markup
Leverages existing test infrastructure for validation
More concise and maintainable

Technical Details

The key challenge was handling position changes across multiple buffer edits:

Spans are processed in reverse order (last to first) to avoid position invalidation of earlier positions
Each split produces a new snapshot and a new caret position
Using raw int positions would be incorrect because earlier edits shift later positions
ITrackingPoint automatically adjusts positions as the buffer changes
Final positions are resolved from tracking points in the final snapshot

Security Summary

✅ No security vulnerabilities detected by CodeQL scanner
✅ All code review feedback addressed
✅ All changes are minimal and focused on the specific issue

Original prompt

This section details on the original issue you should resolve

<issue_title>String splitter incorrectly handles multi-caret scenarios</issue_title>
<issue_description>Hitting enter in the following cases results in odd behavior:
1)

The primary caret is moved to the first location and additional 2 carets are added. I'd expect the primary stay the last and only 3 carets are present after splitting the string.

A couple of carets added that shouldn't be.

There is probably more cases like this.

Seems like the implementation should use IMultiSelectionBroker editor API instead of setting TextView.Caret.

</issue_description>

<agent_instructions>Copilot. It looks like our logic for determining the new text is correct. However, we mess up where teh caret is supposed to go. We currently have logic that bails out from setting the caret in the case where we have multiple initial carets in SplitStringLiteralCommandHandler.

What probably makes sense is to build up the final locations of the carets as we make each edit (or compute it all after we make all the edits). Then use some api to set all the carets at the end.

See IMultiSelectionBroker which could be helpful here. Note: this may be complex since we compute from end to start, and earlier edits could affect the positions of later carets. Maybe this is easy to compute, but maybe not.</agent_instructions>

Comments on the Issue (you are @copilot in this section)

Fixes String splitter incorrectly handles multi-caret scenarios #64812

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

- Track caret positions for all splits - Use SetMultiSelection to set all carets after edits - Remove single-caret-only restriction Co-authored-by: CyrusNajmabadi <4564579+CyrusNajmabadi@users.noreply.github.com>

When processing multiple string splits in reverse order, positions from earlier snapshots need to be tracked forward to the final snapshot. Using ITrackingPoint ensures positions are automatically adjusted as the buffer changes. Co-authored-by: CyrusNajmabadi <4564579+CyrusNajmabadi@users.noreply.github.com>

CyrusNajmabadi · 2025-10-24T14:33:36Z

src/EditorFeatures/CSharp/SplitStringLiteral/SplitStringLiteralCommandHandler.cs

+        // However, we need to track the new caret positions in the original forward order
+        // for setting the multi-selection at the end. We use tracking points to automatically
+        // adjust positions as the buffer changes.
+        var trackingPoints = new ITrackingPoint[spans.Count];


@copilot please do using var _ = ArrayBuilder<ITrackingPoint>.GetInstance(spans.Count, out var trackingPoints); (or whatever the normal pattern is in this code for a pooled array of the right length.

Done in ca3e79c. Changed to use PooledObjects.ArrayBuilder<ITrackingPoint>.GetInstance(spans.Count, out var trackingPoints) with the fully qualified namespace to avoid ambiguity.

CyrusNajmabadi · 2025-10-24T14:34:00Z

src/EditorFeatures/CSharp/SplitStringLiteral/SplitStringLiteralCommandHandler.cs

+        // Now set all the caret positions using the multi-selection broker.
+        // Get the position of each tracking point in the final snapshot.
+        var finalSnapshot = subjectBuffer.CurrentSnapshot;
+        using var pooledSpans = TemporaryArray<SnapshotSpan>.Empty;


@copilot rename this to finalCaretSpans

Done in ca3e79c. Renamed to finalCaretSpans.

Added comprehensive test that validates all caret positions after multi-caret string splitting, not just the primary caret. Co-authored-by: CyrusNajmabadi <4564579+CyrusNajmabadi@users.noreply.github.com>

CyrusNajmabadi · 2025-10-24T14:40:38Z

src/EditorFeatures/CSharpTest/SplitStringLiteral/SplitStringLiteralCommandHandlerTests.cs

+
+        // The carets should be right after the opening quotes of the continuation strings
+        Assert.True(firstSelection.Start.Position >= firstCaretPosition && firstSelection.Start.Position <= firstCaretPosition + 10);
+        Assert.True(secondSelection.Start.Position >= secondCaretPosition && secondSelection.Start.Position <= secondCaretPosition + 10);


@copilot instead of manually computing the positions, put them in the expectedOutput with [||] markers as well. Then use the MarkupTestFile helpers to extract out the text content and caret positions of the expected outcome to validate against.

Done in ca3e79c. Simplified the test to use TestHandled helper which automatically validates both the text output and caret positions using the [||] markers.

- Use ImmutableArray.Length instead of Count - Use GetMultiSelectionBroker().AllSelections for validation - Validate all selection positions, not just text fragments Co-authored-by: CyrusNajmabadi <4564579+CyrusNajmabadi@users.noreply.github.com>

- 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>

…github.com/dotnet/roslyn into copilot/fix-string-splitter-caret-handling

CyrusNajmabadi · 2025-10-24T18:30:15Z

@dibarbet ptal.

Initial plan

325cde8

Copilot AI assigned Copilot and CyrusNajmabadi Oct 24, 2025

Copilot started work on behalf of CyrusNajmabadi October 24, 2025 14:07 View session

Copilot AI and others added 2 commits October 24, 2025 14:26

Implement multi-caret support for string splitting

7a411eb

- Track caret positions for all splits - Use SetMultiSelection to set all carets after edits - Remove single-caret-only restriction Co-authored-by: CyrusNajmabadi <4564579+CyrusNajmabadi@users.noreply.github.com>

CyrusNajmabadi reviewed Oct 24, 2025

View reviewed changes

Add test to verify all multi-caret positions are set correctly

e34eb2e

Added comprehensive test that validates all caret positions after multi-caret string splitting, not just the primary caret. Co-authored-by: CyrusNajmabadi <4564579+CyrusNajmabadi@users.noreply.github.com>

CyrusNajmabadi reviewed Oct 24, 2025

View reviewed changes

Copilot AI changed the title ~~[WIP] Fix string splitter handling of multi-caret scenarios~~ Fix string splitter multi-caret handling Oct 24, 2025

Copilot AI requested a review from CyrusNajmabadi October 24, 2025 14:43

Copilot finished work on behalf of CyrusNajmabadi October 24, 2025 14:43

Copilot started work on behalf of CyrusNajmabadi October 24, 2025 14:44 View session

Address code review feedback

ca3e79c

- 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>

Copilot finished work on behalf of CyrusNajmabadi October 24, 2025 14:51

CyrusNajmabadi added 3 commits October 24, 2025 09:54

Fix test

a4ee761

Merge branch 'copilot/fix-string-splitter-caret-handling' of https://…

f258d3b

…github.com/dotnet/roslyn into copilot/fix-string-splitter-caret-handling

Remove usings

9ae325e

CyrusNajmabadi marked this pull request as ready for review October 24, 2025 14:59

CyrusNajmabadi requested a review from a team as a code owner October 24, 2025 14:59

CyrusNajmabadi requested a review from dibarbet October 24, 2025 18:30

dibarbet approved these changes Oct 24, 2025

View reviewed changes

CyrusNajmabadi enabled auto-merge (rebase) October 24, 2025 18:35

CyrusNajmabadi disabled auto-merge October 24, 2025 18:35

CyrusNajmabadi merged commit a6c772c into main Oct 24, 2025
25 checks passed

CyrusNajmabadi deleted the copilot/fix-string-splitter-caret-handling branch October 24, 2025 18:35

dotnet-policy-service bot added this to the Next milestone Oct 24, 2025

dotnet-maestro bot mentioned this pull request Oct 29, 2025

[main] Source code updates from dotnet/roslyn dotnet/dotnet#3147

Closed

This was referenced Oct 30, 2025

[Automated] PRs inserted in VS build main-11129.33 #80956

Closed

[Automated] PRs inserted in VS build feature.debugger.main-11131.247 #80976

Closed

dotnet-maestro bot mentioned this pull request Dec 18, 2025

[main] Source code updates from dotnet/roslyn dotnet/dotnet#3957

Merged

davidwengier modified the milestones: Next, 18.3 Jan 6, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix string splitter multi-caret handling#80883

Fix string splitter multi-caret handling#80883
CyrusNajmabadi merged 9 commits intomainfrom
copilot/fix-string-splitter-caret-handling

Copilot AI commented Oct 24, 2025 •

edited

Loading

Uh oh!

CyrusNajmabadi Oct 24, 2025

Uh oh!

Copilot AI Oct 24, 2025

Uh oh!

CyrusNajmabadi Oct 24, 2025

Uh oh!

Copilot AI Oct 24, 2025

Uh oh!

CyrusNajmabadi Oct 24, 2025

Uh oh!

Copilot AI Oct 24, 2025

Uh oh!

CyrusNajmabadi commented Oct 24, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

Copilot AI commented Oct 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Plan

Summary

Changes Made

Technical Details

Security Summary

Comments on the Issue (you are @copilot in this section)

Uh oh!

CyrusNajmabadi Oct 24, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 24, 2025

Choose a reason for hiding this comment

Uh oh!

CyrusNajmabadi Oct 24, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 24, 2025

Choose a reason for hiding this comment

Uh oh!

CyrusNajmabadi Oct 24, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 24, 2025

Choose a reason for hiding this comment

Uh oh!

CyrusNajmabadi commented Oct 24, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Copilot AI commented Oct 24, 2025 •

edited

Loading