From ac61f2642f1967c184831c571e1649f9a27e7a2b Mon Sep 17 00:00:00 2001 From: dino Date: Tue, 23 Dec 2025 20:39:05 +0000 Subject: [PATCH] fix(editor): use buffer columns when adding selections 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 ```` --- 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(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index d985a4d269f2eaeb3fa6056192095b7913b579b6..ca5eee4794b190156aaebd8387062874676ea2f5 100644 --- a/crates/editor/src/editor.rs +++ b/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; } diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index dc990d69f218dad083c888f75819cadae3f22cb1..b48395afc4b94238125a2e27b753a6aaf1b2f9ba 100644 --- a/crates/editor/src/editor_tests.rs +++ b/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::(&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] diff --git a/crates/editor/src/selections_collection.rs b/crates/editor/src/selections_collection.rs index 54bb7ceec1d035fbefb0c229c4e537e8277b67cd..58904f64cd4914eac6ef78b8a6382f01d1457b08 100644 --- a/crates/editor/src/selections_collection.rs +++ b/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, + reversed: bool, + text_layout_details: &TextLayoutDetails, + ) -> Option> { + 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( &mut self, snapshot: &DisplaySnapshot,