editor: Fix multi-cursor not added to lines shorter than current cursor column (#31100)

smit created 6 months ago

Closes #5255, #1046, #28322, #15728

This PR makes `AddSelectionBelow` and `AddSelectionAbove` not skip lines
that are shorter than the current cursor column. This follows the same
behavior as VSCode and Sublime.

This change is only applicable in the case of an empty selection; if
there is a non-empty selection, it continues to skip empty and shorter
lines to create a Vim-like column selection, which is the better default
for that case.

- [x] Tests

The empty selection no longer skips shorter lines:


https://github.com/user-attachments/assets/4bde2357-20b6-44f2-a9d9-b595c12d3939

Non-empty selection continues to skip shorter lines.


https://github.com/user-attachments/assets/4cd47c9f-b698-40fc-ad50-f2bf64f5519b

Release Notes:

- Improved `AddSelectionBelow` and `AddSelectionAbove` to no longer skip
shorter lines when the selection is empty, aligning with VSCode and
Sublime behavior.

Change summary

crates/editor/src/editor_tests.rs          | 32 +++++++++++++++++++--
crates/editor/src/selections_collection.rs | 36 +++++++++++++----------
2 files changed, 49 insertions(+), 19 deletions(-)

Detailed changes

crates/editor/src/editor_tests.rs

@@ -5965,9 +5965,9 @@ async fn test_add_selection_above_below(cx: &mut TestAppContext) {
     cx.assert_editor_state(indoc!(
         r#"abc
            defˇghi
-
+           ˇ
            jk
-           nlmˇo
+           nlmo
            "#
     ));
 
@@ -5978,12 +5978,38 @@ async fn test_add_selection_above_below(cx: &mut TestAppContext) {
     cx.assert_editor_state(indoc!(
         r#"abc
            defˇghi
+           ˇ
+           jkˇ
+           nlmo
+           "#
+    ));
 
-           jk
+    cx.update_editor(|editor, window, cx| {
+        editor.add_selection_below(&Default::default(), window, cx);
+    });
+
+    cx.assert_editor_state(indoc!(
+        r#"abc
+           defˇghi
+           ˇ
+           jkˇ
            nlmˇo
            "#
     ));
 
+    cx.update_editor(|editor, window, cx| {
+        editor.add_selection_below(&Default::default(), window, cx);
+    });
+
+    cx.assert_editor_state(indoc!(
+        r#"abc
+           defˇghi
+           ˇ
+           jkˇ
+           nlmˇo
+           ˇ"#
+    ));
+
     // change selections
     cx.set_state(indoc!(
         r#"abc

crates/editor/src/selections_collection.rs

@@ -352,28 +352,32 @@ impl SelectionsCollection {
     ) -> Option<Selection<Point>> {
         let is_empty = positions.start == positions.end;
         let line_len = display_map.line_len(row);
-
         let line = display_map.layout_row(row, text_layout_details);
-
         let start_col = line.closest_index_for_x(positions.start) as u32;
-        if start_col < line_len || (is_empty && positions.start == line.width) {
+
+        let (start, end) = if is_empty {
+            let point = DisplayPoint::new(row, std::cmp::min(start_col, line_len));
+            (point, point)
+        } else {
+            if start_col >= line_len {
+                return None;
+            }
             let start = DisplayPoint::new(row, start_col);
             let end_col = line.closest_index_for_x(positions.end) as u32;
             let end = DisplayPoint::new(row, end_col);
+            (start, end)
+        };
 
-            Some(Selection {
-                id: post_inc(&mut self.next_selection_id),
-                start: start.to_point(display_map),
-                end: end.to_point(display_map),
-                reversed,
-                goal: SelectionGoal::HorizontalRange {
-                    start: positions.start.into(),
-                    end: positions.end.into(),
-                },
-            })
-        } else {
-            None
-        }
+        Some(Selection {
+            id: post_inc(&mut self.next_selection_id),
+            start: start.to_point(display_map),
+            end: end.to_point(display_map),
+            reversed,
+            goal: SelectionGoal::HorizontalRange {
+                start: positions.start.into(),
+                end: positions.end.into(),
+            },
+        })
     }
 
     pub fn change_with<R>(