From edae88c223fafa174288bdf112becf1cecf7837e Mon Sep 17 00:00:00 2001 From: Dino Date: Thu, 15 Jan 2026 18:28:46 +0000 Subject: [PATCH] editor: Fix add selection when skipping over soft-wrapped lines (#46911) Fixes an issue where `editor::AddSelectionAbove` and `editor::AddSelectionBelow`, when set with `skip_soft_wrap: true` , would lose track of the original cursor column when passing through short lines, as `editor::Editor.add_selection` was not using the oldest selection's column range when adding a new selection. These changes update the `editor::Editor.add_selection` method so as to keep track of the olde selection's column range for each group, as well as extracting `find_next_columnar_selection_by_display_row` and `find_next_columnar_selection_by_buffer_row` helper methods to clarify the two navigation strategies. Closes #46842 Release Notes: - Fixed `editor::AddSelectionAbove` and `editor::AddSelectionBelow` when skipping over soft-wrapped lines to preserve the original cursor column when adding selections through lines shorter than the starting position --- crates/editor/src/editor.rs | 102 +++++++++++---------- crates/editor/src/editor_tests.rs | 78 ++++++++++++++++ crates/editor/src/selections_collection.rs | 66 +++++++++++++ 3 files changed, 199 insertions(+), 47 deletions(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index aa15a57a45c9da6ebe35be7305a2e1169bd12c3e..a59f71053839766b998c45a37881903c18bc3a10 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -15141,6 +15141,32 @@ impl Editor { display_map.max_point().row() }; + // When `skip_soft_wrap` is true, we use buffer columns instead of pixel + // positions to place new selections, so we need to keep track of the + // column range of the oldest selection in each group, because + // intermediate selections may have been clamped to shorter lines. + // selections may have been clamped to shorter lines. + let mut goal_columns_by_selection_id = if skip_soft_wrap { + let mut map = HashMap::default(); + for group in state.groups.iter() { + if let Some(oldest_id) = group.stack.first() { + if let Some(oldest_selection) = + columnar_selections.iter().find(|s| s.id == *oldest_id) + { + let start_col = oldest_selection.start.column; + let end_col = oldest_selection.end.column; + let goal_columns = start_col.min(end_col)..start_col.max(end_col); + for id in &group.stack { + map.insert(*id, goal_columns.clone()); + } + } + } + } + map + } else { + HashMap::default() + }; + let mut last_added_item_per_group = HashMap::default(); for group in state.groups.iter_mut() { if let Some(last_id) = group.stack.last() { @@ -15153,7 +15179,7 @@ impl Editor { if above == group.above { let range = selection.display_range(&display_map).sorted(); debug_assert_eq!(range.start.row(), range.end.row()); - let mut row = range.start.row(); + let row = range.start.row(); let positions = if let SelectionGoal::HorizontalRange { start, end } = selection.goal { Pixels::from(start)..Pixels::from(end) @@ -15165,52 +15191,34 @@ impl Editor { start_x.min(end_x)..start_x.max(end_x) }; - let mut maybe_new_selection = None; - let direction = if above { -1 } else { 1 }; - - while row != end_row { - 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 { - 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) = new_selection { - maybe_new_selection = Some(new_selection); - break; - } - } + let maybe_new_selection = if skip_soft_wrap { + let goal_columns = goal_columns_by_selection_id + .remove(&selection.id) + .unwrap_or_else(|| { + let start_col = selection.start.column; + let end_col = selection.end.column; + start_col.min(end_col)..start_col.max(end_col) + }); + self.selections.find_next_columnar_selection_by_buffer_row( + &display_map, + row, + end_row, + above, + &goal_columns, + selection.reversed, + &text_layout_details, + ) + } else { + self.selections.find_next_columnar_selection_by_display_row( + &display_map, + row, + end_row, + above, + &positions, + selection.reversed, + &text_layout_details, + ) + }; if let Some(new_selection) = maybe_new_selection { group.stack.push(new_selection.id); diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index e805cfb0abc239f34ef645805a8e1d40df7494b3..b042c65f6df6b36843b2d0b45b26b730c34e47c7 100644 --- a/crates/editor/src/editor_tests.rs +++ b/crates/editor/src/editor_tests.rs @@ -8364,6 +8364,84 @@ async fn test_add_selection_above_below(cx: &mut TestAppContext) { n«ˇlm»o "# )); + + // Assert that the oldest selection's goal column is used when adding more + // selections, not the most recently added selection's actual column. + cx.set_state(indoc! {" + foo bar bazˇ + foo + foo bar + "}); + + cx.update_editor(|editor, window, cx| { + editor.add_selection_below( + &AddSelectionBelow { + skip_soft_wrap: true, + }, + window, + cx, + ); + }); + + cx.assert_editor_state(indoc! {" + foo bar bazˇ + fooˇ + foo bar + "}); + + cx.update_editor(|editor, window, cx| { + editor.add_selection_below( + &AddSelectionBelow { + skip_soft_wrap: true, + }, + window, + cx, + ); + }); + + cx.assert_editor_state(indoc! {" + foo bar bazˇ + fooˇ + foo barˇ + "}); + + cx.set_state(indoc! {" + foo bar baz + foo + foo barˇ + "}); + + cx.update_editor(|editor, window, cx| { + editor.add_selection_above( + &AddSelectionAbove { + skip_soft_wrap: true, + }, + window, + cx, + ); + }); + + cx.assert_editor_state(indoc! {" + foo bar baz + fooˇ + foo barˇ + "}); + + cx.update_editor(|editor, window, cx| { + editor.add_selection_above( + &AddSelectionAbove { + skip_soft_wrap: true, + }, + window, + cx, + ); + }); + + cx.assert_editor_state(indoc! {" + foo barˇ baz + fooˇ + foo barˇ + "}); } #[gpui::test] diff --git a/crates/editor/src/selections_collection.rs b/crates/editor/src/selections_collection.rs index 58904f64cd4914eac6ef78b8a6382f01d1457b08..63739bcef115163e953e458edbf4e09eec176577 100644 --- a/crates/editor/src/selections_collection.rs +++ b/crates/editor/src/selections_collection.rs @@ -461,6 +461,72 @@ impl SelectionsCollection { }) } + /// Finds the next columnar selection by walking display rows one at a time + /// so that soft-wrapped lines are considered and not skipped. + pub fn find_next_columnar_selection_by_display_row( + &mut self, + display_map: &DisplaySnapshot, + start_row: DisplayRow, + end_row: DisplayRow, + above: bool, + positions: &Range, + reversed: bool, + text_layout_details: &TextLayoutDetails, + ) -> Option> { + let mut row = start_row; + while row != end_row { + if above { + row.0 -= 1; + } else { + row.0 += 1; + } + + if let Some(selection) = self.build_columnar_selection( + display_map, + row, + positions, + reversed, + text_layout_details, + ) { + return Some(selection); + } + } + None + } + + /// Finds the next columnar selection by skipping to the next buffer row, + /// ignoring soft-wrapped lines. + pub fn find_next_columnar_selection_by_buffer_row( + &mut self, + display_map: &DisplaySnapshot, + start_row: DisplayRow, + end_row: DisplayRow, + above: bool, + goal_columns: &Range, + reversed: bool, + text_layout_details: &TextLayoutDetails, + ) -> Option> { + let mut row = start_row; + let direction = if above { -1 } else { 1 }; + while row != end_row { + let new_row = + display_map.start_of_relative_buffer_row(DisplayPoint::new(row, 0), direction); + row = new_row.row(); + let buffer_row = new_row.to_point(display_map).row; + + if let Some(selection) = self.build_columnar_selection_from_buffer_columns( + display_map, + buffer_row, + goal_columns, + reversed, + text_layout_details, + ) { + return Some(selection); + } + } + None + } + pub fn change_with( &mut self, snapshot: &DisplaySnapshot,