Skip to content

[diffs/editor] Fix some selection bug#906

Open
ije wants to merge 3 commits into
beta-1.3from
editor/fix-multiple-selection
Open

[diffs/editor] Fix some selection bug#906
ije wants to merge 3 commits into
beta-1.3from
editor/fix-multiple-selection

Conversation

@ije

@ije ije commented Jun 29, 2026

Copy link
Copy Markdown
Collaborator
  • Fix cursor movement handling for backward multi-line selections
  • Fix selection merging for multiple cursors with shift key pressed

@ije ije requested a review from necolas June 29, 2026 17:01
@vercel

vercel Bot commented Jun 29, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
pierre-docs-diffs Ready Ready Preview Jun 29, 2026 7:14pm
pierre-docs-diffshub Ready Ready Preview Jun 29, 2026 7:14pm
pierre-docs-trees Ready Ready Preview Jun 29, 2026 7:14pm
pierrejs-diff-demo Ready Ready Preview Jun 29, 2026 7:14pm

Request Review

When overlapping selections merge, the result takes its direction from
the most recently created selection, so the merged caret stays where
the user last put it. Two paths had no tests: when the latest selection
is already backward, and when a later bare caret lands on the merged
start and the direction is derived as backward.

Add a test for each so this focus-preserving behavior is locked in.
@necolas

necolas commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

LGTM, nice fix.

I pushed a test-only commit (70ccda0) on top of your branch; no source changes.

While reviewing, the new direction handling in mergeOverlappingSelections had two branches that weren't exercised by tests:

  • the most-recent overlapping selection is already backward → the merged range should keep that backward direction (the direction = latest.selection.direction path)
  • a later bare caret lands on the merged start → the direction is derived as backward (the DirectionNone re-derivation path)

Both exist so the merged caret stays where it was most recently placed, so I added one test for each to pin that focus-preservation behavior down.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants