From feb3ce3638f4be201ef8ab8b6e6c4f4f38fd0b1a Mon Sep 17 00:00:00 2001 From: Dino Date: Mon, 5 Jan 2026 10:33:11 +0000 Subject: [PATCH] User buffer column range when adding selection that skips soft wrap (#45594) 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 ```` Closes #42137 Release Notes: - Improved `editor: add selection above` and `editor: add selection below` to select at the same text column when skipping soft-wrapped lines, instead of the same visual position --- 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 985b5d6bb31125a7d1bab7a986996be78ef96063..ac077844a634ed4f29d04e8f2878717b25393a4b 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -14995,23 +14995,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 cb1f2ca3334d3d2bf01fee76fbf2dfbe7a57c4f4..73487bc43a776a22141eb77308df8f22a783fb30 100644 --- a/crates/editor/src/editor_tests.rs +++ b/crates/editor/src/editor_tests.rs @@ -27315,6 +27315,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,