fix(editor): use buffer columns when adding selections

dino created

When adding selections above/below with `skip_soft_wrap: true`, use buffer
column positions instead of pixel positions. This ensures selections are
placed at the same column offset in the buffer rather than at the same
visual position, which was incorrect for soft-wrapped lines.

For example, selecting "how" in a wrapped line:

````
1. Very long line to
   show [how] a wrapped
   line would look
2. Very long line to
   show how a wrapped
   line would look
````

Now correctly adds a selection at the same buffer column in the next
line:

````
1. Very long line to
   show [how] a wrapped
   line would look
2. Very long line to
   show [how] a wrapped
   line would look
````

Change summary

crates/editor/src/editor.rs                | 51 ++++++++++++++++-------
crates/editor/src/editor_tests.rs          | 32 +++++++++++++++
crates/editor/src/selections_collection.rs | 50 +++++++++++++++++++++++
3 files changed, 118 insertions(+), 15 deletions(-)

Detailed changes

crates/editor/src/editor.rs ๐Ÿ”—

@@ -14915,23 +14915,44 @@ impl Editor {
                     let direction = if above { -1 } else { 1 };
 
                     while row != end_row {
-                        if skip_soft_wrap {
-                            row = display_map
-                                .start_of_relative_buffer_row(DisplayPoint::new(row, 0), direction)
-                                .row();
-                        } else if above {
-                            row.0 -= 1;
+                        let new_buffer_row = if skip_soft_wrap {
+                            let new_row = display_map
+                                .start_of_relative_buffer_row(DisplayPoint::new(row, 0), direction);
+                            row = new_row.row();
+                            Some(new_row.to_point(&display_map).row)
                         } else {
-                            row.0 += 1;
-                        }
+                            if above {
+                                row.0 -= 1;
+                            } else {
+                                row.0 += 1;
+                            }
+                            None
+                        };
+
+                        let new_selection = if let Some(buffer_row) = new_buffer_row {
+                            let start_col = selection.start.column;
+                            let end_col = selection.end.column;
+                            let buffer_columns = start_col.min(end_col)..start_col.max(end_col);
+
+                            self.selections
+                                .build_columnar_selection_from_buffer_columns(
+                                    &display_map,
+                                    buffer_row,
+                                    &buffer_columns,
+                                    selection.reversed,
+                                    &text_layout_details,
+                                )
+                        } else {
+                            self.selections.build_columnar_selection(
+                                &display_map,
+                                row,
+                                &positions,
+                                selection.reversed,
+                                &text_layout_details,
+                            )
+                        };
 
-                        if let Some(new_selection) = self.selections.build_columnar_selection(
-                            &display_map,
-                            row,
-                            &positions,
-                            selection.reversed,
-                            &text_layout_details,
-                        ) {
+                        if let Some(new_selection) = new_selection {
                             maybe_new_selection = Some(new_selection);
                             break;
                         }

crates/editor/src/editor_tests.rs ๐Ÿ”—

@@ -27311,6 +27311,38 @@ async fn test_add_selection_skip_soft_wrap_option(cx: &mut TestAppContext) {
             &[DisplayPoint::new(DisplayRow(0), 0)..DisplayPoint::new(DisplayRow(0), 0)]
         );
     });
+
+    // Set up text where selections are in the middle of a soft-wrapped line.
+    // When adding selection below with `skip_soft_wrap` set to `true`, the new
+    // selection should be at the same buffer column, not the same pixel
+    // position.
+    cx.set_state(indoc!(
+        r#"1. Very long line to show ยซhowห‡ยป a wrapped line would look
+           2. Very long line to show how a wrapped line would look"#
+    ));
+
+    cx.update_editor(|editor, window, cx| {
+        // Enable soft wrapping with a narrow width to force soft wrapping and
+        // confirm that more than 2 rows are being displayed.
+        editor.set_wrap_width(Some(100.0.into()), cx);
+        assert!(editor.display_text(cx).lines().count() > 2);
+
+        editor.add_selection_below(
+            &AddSelectionBelow {
+                skip_soft_wrap: true,
+            },
+            window,
+            cx,
+        );
+
+        // Assert that there's now 2 selections, both selecting the same column
+        // range in the buffer row.
+        let display_map = editor.display_map.update(cx, |map, cx| map.snapshot(cx));
+        let selections = editor.selections.all::<Point>(&display_map);
+        assert_eq!(selections.len(), 2);
+        assert_eq!(selections[0].start.column, selections[1].start.column);
+        assert_eq!(selections[0].end.column, selections[1].end.column);
+    });
 }
 
 #[gpui::test]

crates/editor/src/selections_collection.rs ๐Ÿ”—

@@ -411,6 +411,56 @@ impl SelectionsCollection {
         })
     }
 
+    /// Attempts to build a selection in the provided buffer row using the
+    /// same buffer column range as specified.
+    /// Returns `None` if the range is not empty but it starts past the line's
+    /// length, meaning that the line isn't long enough to be contained within
+    /// part of the provided range.
+    pub fn build_columnar_selection_from_buffer_columns(
+        &mut self,
+        display_map: &DisplaySnapshot,
+        buffer_row: u32,
+        positions: &Range<u32>,
+        reversed: bool,
+        text_layout_details: &TextLayoutDetails,
+    ) -> Option<Selection<Point>> {
+        let is_empty = positions.start == positions.end;
+        let line_len = display_map
+            .buffer_snapshot()
+            .line_len(multi_buffer::MultiBufferRow(buffer_row));
+
+        let (start, end) = if is_empty {
+            let column = std::cmp::min(positions.start, line_len);
+            let point = Point::new(buffer_row, column);
+            (point, point)
+        } else {
+            if positions.start >= line_len {
+                return None;
+            }
+
+            let start = Point::new(buffer_row, positions.start);
+            let end_column = std::cmp::min(positions.end, line_len);
+            let end = Point::new(buffer_row, end_column);
+            (start, end)
+        };
+
+        let start_display_point = start.to_display_point(display_map);
+        let end_display_point = end.to_display_point(display_map);
+        let start_x = display_map.x_for_display_point(start_display_point, text_layout_details);
+        let end_x = display_map.x_for_display_point(end_display_point, text_layout_details);
+
+        Some(Selection {
+            id: post_inc(&mut self.next_selection_id),
+            start,
+            end,
+            reversed,
+            goal: SelectionGoal::HorizontalRange {
+                start: start_x.min(end_x).into(),
+                end: start_x.max(end_x).into(),
+            },
+        })
+    }
+
     pub fn change_with<R>(
         &mut self,
         snapshot: &DisplaySnapshot,