editor: Don't merge adjacent selections (#44811)

Marco Mihai Condrache created

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

Change summary

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

Detailed changes

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();
         }
     "});
 

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::<Vec<_>>();
         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<D: Ord + fmt::Debug + Copy>(
     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<D: Ord + fmt::Debug + Copy>(
         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<T: Ord + Copy>(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
+}

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,