From 34122aeb21626bfc0502ab7c768ccee2c2dde392 Mon Sep 17 00:00:00 2001 From: Marco Mihai Condrache <52580954+marcocondrache@users.noreply.github.com> Date: Mon, 15 Dec 2025 17:32:54 +0100 Subject: [PATCH] editor: Don't merge adjacent selections (#44811) Closes #24748 Release Notes: - Adjacent selections are not merged anymore --------- Signed-off-by: Marco Mihai Condrache <52580954+marcocondrache@users.noreply.github.com> Signed-off-by: Marco Mihai Condrache --- crates/editor/src/editor_tests.rs | 20 ++++--- crates/editor/src/selections_collection.rs | 63 ++++++++++++++++++++-- crates/vim/src/helix.rs | 3 +- 3 files changed, 73 insertions(+), 13 deletions(-) diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index 89131e8bc39fc03e54e19c9d8b1f79a7f2d66cb9..9b04a6ea2bc7aef4e5a90b7d823e50857cff2172 100644 --- a/crates/editor/src/editor_tests.rs +++ b/crates/editor/src/editor_tests.rs @@ -7750,10 +7750,12 @@ fn test_select_line(cx: &mut TestAppContext) { ]) }); editor.select_line(&SelectLine, window, cx); + // Adjacent line selections should NOT merge (only overlapping ones do) assert_eq!( display_ranges(editor, cx), vec![ - DisplayPoint::new(DisplayRow(0), 0)..DisplayPoint::new(DisplayRow(2), 0), + DisplayPoint::new(DisplayRow(0), 0)..DisplayPoint::new(DisplayRow(1), 0), + DisplayPoint::new(DisplayRow(1), 0)..DisplayPoint::new(DisplayRow(2), 0), DisplayPoint::new(DisplayRow(4), 0)..DisplayPoint::new(DisplayRow(5), 0), ] ); @@ -7772,9 +7774,13 @@ fn test_select_line(cx: &mut TestAppContext) { _ = editor.update(cx, |editor, window, cx| { editor.select_line(&SelectLine, window, cx); + // Adjacent but not overlapping, so they stay separate assert_eq!( display_ranges(editor, cx), - vec![DisplayPoint::new(DisplayRow(0), 0)..DisplayPoint::new(DisplayRow(5), 5)] + vec![ + DisplayPoint::new(DisplayRow(0), 0)..DisplayPoint::new(DisplayRow(4), 0), + DisplayPoint::new(DisplayRow(4), 0)..DisplayPoint::new(DisplayRow(5), 5), + ] ); }); } @@ -16196,7 +16202,7 @@ async fn test_toggle_comment(cx: &mut TestAppContext) { cx.assert_editor_state(indoc! {" fn a() { «b(); - c(); + ˇ»«c(); ˇ» d(); } "}); @@ -16208,8 +16214,8 @@ async fn test_toggle_comment(cx: &mut TestAppContext) { cx.assert_editor_state(indoc! {" fn a() { // «b(); - // c(); - ˇ»// d(); + ˇ»// «c(); + ˇ» // d(); } "}); @@ -16218,7 +16224,7 @@ async fn test_toggle_comment(cx: &mut TestAppContext) { fn a() { // b(); «// c(); - ˇ» // d(); + ˇ» // d(); } "}); @@ -16228,7 +16234,7 @@ async fn test_toggle_comment(cx: &mut TestAppContext) { fn a() { // b(); «c(); - ˇ» // d(); + ˇ» // d(); } "}); diff --git a/crates/editor/src/selections_collection.rs b/crates/editor/src/selections_collection.rs index 6f744e11334fc32e7985ee77e25866ef0c6cfe4c..54bb7ceec1d035fbefb0c229c4e537e8277b67cd 100644 --- a/crates/editor/src/selections_collection.rs +++ b/crates/editor/src/selections_collection.rs @@ -136,7 +136,13 @@ impl SelectionsCollection { iter::from_fn(move || { if let Some(pending) = pending_opt.as_mut() { while let Some(next_selection) = disjoint.peek() { - if pending.start <= next_selection.end && pending.end >= next_selection.start { + if should_merge( + pending.start, + pending.end, + next_selection.start, + next_selection.end, + false, + ) { let next_selection = disjoint.next().unwrap(); if next_selection.start < pending.start { pending.start = next_selection.start; @@ -236,7 +242,13 @@ impl SelectionsCollection { iter::from_fn(move || { if let Some(pending) = pending_opt.as_mut() { while let Some(next_selection) = disjoint.peek() { - if pending.start <= next_selection.end && pending.end >= next_selection.start { + if should_merge( + pending.start, + pending.end, + next_selection.start, + next_selection.end, + false, + ) { let next_selection = disjoint.next().unwrap(); if next_selection.start < pending.start { pending.start = next_selection.start; @@ -666,10 +678,13 @@ impl<'snap, 'a> MutableSelectionsCollection<'snap, 'a> { }) .collect::>(); selections.sort_unstable_by_key(|s| s.start); - // Merge overlapping selections. + let mut i = 1; while i < selections.len() { - if selections[i].start <= selections[i - 1].end { + let prev = &selections[i - 1]; + let current = &selections[i]; + + if should_merge(prev.start, prev.end, current.start, current.end, true) { let removed = selections.remove(i); if removed.start < selections[i - 1].start { selections[i - 1].start = removed.start; @@ -1139,7 +1154,13 @@ fn coalesce_selections( iter::from_fn(move || { let mut selection = selections.next()?; while let Some(next_selection) = selections.peek() { - if selection.end >= next_selection.start { + if should_merge( + selection.start, + selection.end, + next_selection.start, + next_selection.end, + true, + ) { if selection.reversed == next_selection.reversed { selection.end = cmp::max(selection.end, next_selection.end); selections.next(); @@ -1161,3 +1182,35 @@ fn coalesce_selections( Some(selection) }) } + +/// Determines whether two selections should be merged into one. +/// +/// Two selections should be merged when: +/// 1. They overlap: the selections share at least one position +/// 2. They have the same start position: one contains or equals the other +/// 3. A cursor touches a selection boundary: a zero-width selection (cursor) at the +/// start or end of another selection should be absorbed into it +/// +/// Note: two selections that merely touch (one ends exactly where the other begins) +/// but don't share any positions remain separate, see: https://github.com/zed-industries/zed/issues/24748 +fn should_merge(a_start: T, a_end: T, b_start: T, b_end: T, sorted: bool) -> bool { + let is_overlapping = if sorted { + // When sorted, `a` starts before or at `b`, so overlap means `b` starts before `a` ends + b_start < a_end + } else { + a_start < b_end && b_start < a_end + }; + + // Selections starting at the same position should always merge (one contains the other) + let same_start = a_start == b_start; + + // A cursor (zero-width selection) touching another selection's boundary should merge. + // This handles cases like a cursor at position X merging with a selection that + // starts or ends at X. + let is_cursor_a = a_start == a_end; + let is_cursor_b = b_start == b_end; + let cursor_at_boundary = (is_cursor_a && (a_start == b_start || a_end == b_end)) + || (is_cursor_b && (b_start == a_start || b_end == a_end)); + + is_overlapping || same_start || cursor_at_boundary +} diff --git a/crates/vim/src/helix.rs b/crates/vim/src/helix.rs index fae2bda578c6844c33290d059248b895ebde4c3d..f902a8ff6e9f08475fb6ce8323a924730d3621d1 100644 --- a/crates/vim/src/helix.rs +++ b/crates/vim/src/helix.rs @@ -1389,11 +1389,12 @@ mod test { Mode::HelixNormal, ); cx.simulate_keystrokes("x"); + // Adjacent line selections stay separate (not merged) cx.assert_state( indoc! {" «line one line two - line three + ˇ»«line three line four ˇ»line five"}, Mode::HelixNormal,