From a2f1703e288d88b4ac8ab95f4afe6999be59d2c0 Mon Sep 17 00:00:00 2001 From: Ben Vollrath <116082598+Ben-Vollrath@users.noreply.github.com> Date: Fri, 27 Mar 2026 11:44:46 +0100 Subject: [PATCH] editor: Autoscroll to initial selection on select all matches edit (#49232) Fix the way selections are built in `Editor::select_all_matches` in order to guarantee that the original selection, where the user's cursor is located, is the last selection provided to `MutableSelectionsCollection::select_ranges` as the editor will attempt to scroll to the last selection when the user starts editing. This way, we ensure that the user stays in the same location from which the `editor: select all matches` action was triggered when they start editing. Closes #32894 Release Notes: - Fixed an issue where editing selections after `editor: select all matches` would scroll to the last match --------- Co-authored-by: dino --- crates/editor/src/editor.rs | 41 ++++++++++++--------- crates/editor/src/editor_tests.rs | 60 ++++++++++++++++++++++++++----- 2 files changed, 77 insertions(+), 24 deletions(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 3c35175002fe17995db478f01eed06585c0ebe88..8379191e14c69593430133499eded32da94f8c62 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -16119,11 +16119,8 @@ impl Editor { }; let mut new_selections = Vec::new(); - - let reversed = self - .selections - .oldest::(&display_map) - .reversed; + let initial_selection = self.selections.oldest::(&display_map); + let reversed = initial_selection.reversed; let buffer = display_map.buffer_snapshot(); let query_matches = select_next_state .query @@ -16137,21 +16134,33 @@ impl Editor { MultiBufferOffset(query_match.start())..MultiBufferOffset(query_match.end()) }; - if !select_next_state.wordwise - || (!buffer.is_inside_word(offset_range.start, None) - && !buffer.is_inside_word(offset_range.end, None)) - { - new_selections.push(offset_range.start..offset_range.end); - } - } + let is_partial_word_match = select_next_state.wordwise + && (buffer.is_inside_word(offset_range.start, None) + || buffer.is_inside_word(offset_range.end, None)); - select_next_state.done = true; + let is_initial_selection = MultiBufferOffset(query_match.start()) + == initial_selection.start + && MultiBufferOffset(query_match.end()) == initial_selection.end; - if new_selections.is_empty() { - log::error!("bug: new_selections is empty in select_all_matches"); - return Ok(()); + if !is_partial_word_match && !is_initial_selection { + new_selections.push(offset_range); + } } + // Ensure that the initial range is the last selection, as + // `MutableSelectionsCollection::select_ranges` makes the last selection + // the newest selection, which the editor then relies on as the primary + // cursor for scroll targeting. Without this, the last match would then + // be automatically focused when the user started editing the selected + // matches. + let initial_directed_range = if reversed { + initial_selection.end..initial_selection.start + } else { + initial_selection.start..initial_selection.end + }; + new_selections.push(initial_directed_range); + + select_next_state.done = true; self.unfold_ranges(&new_selections, false, false, cx); self.change_selections(SelectionEffects::no_scroll(), window, cx, |selections| { selections.select_ranges(new_selections) diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index f285d130be5be071b75161c114da53ca9c55d301..79f470d42496c275f3dd729aa9d21cf7cd84c168 100644 --- a/crates/editor/src/editor_tests.rs +++ b/crates/editor/src/editor_tests.rs @@ -9968,7 +9968,6 @@ async fn test_select_all_matches_does_not_scroll(cx: &mut TestAppContext) { init_test(cx, |_| {}); let mut cx = EditorTestContext::new(cx).await; - let large_body_1 = "\nd".repeat(200); let large_body_2 = "\ne".repeat(200); @@ -9981,17 +9980,62 @@ async fn test_select_all_matches_does_not_scroll(cx: &mut TestAppContext) { scroll_position }); - cx.update_editor(|e, window, cx| e.select_all_matches(&SelectAllMatches, window, cx)) + cx.update_editor(|editor, window, cx| editor.select_all_matches(&SelectAllMatches, window, cx)) .unwrap(); cx.assert_editor_state(&format!( "«ˇa»bc\n«ˇa»bc{large_body_1} «ˇa»bc{large_body_2}\nef«ˇa»bc\n«ˇa»bc" )); - let scroll_position_after_selection = - cx.update_editor(|editor, _, cx| editor.scroll_position(cx)); - assert_eq!( - initial_scroll_position, scroll_position_after_selection, - "Scroll position should not change after selecting all matches" - ); + cx.update_editor(|editor, _, cx| { + assert_eq!( + editor.scroll_position(cx), + initial_scroll_position, + "Scroll position should not change after selecting all matches" + ) + }); + + // Simulate typing while the selections are active, as that is where the + // editor would attempt to actually scroll to the newest selection, which + // should have been set as the original selection to avoid scrolling to the + // last match. + cx.simulate_keystroke("x"); + cx.update_editor(|editor, _, cx| { + assert_eq!( + editor.scroll_position(cx), + initial_scroll_position, + "Scroll position should not change after editing all matches" + ) + }); + + cx.set_state(&format!( + "abc\nabc{large_body_1} «aˇ»bc{large_body_2}\nefabc\nabc" + )); + let initial_scroll_position = cx.update_editor(|editor, _, cx| { + let scroll_position = editor.scroll_position(cx); + assert!(scroll_position.y > 0.0, "Initial selection is between two large bodies and should have the editor scrolled to it"); + scroll_position + }); + + cx.update_editor(|editor, window, cx| editor.select_all_matches(&SelectAllMatches, window, cx)) + .unwrap(); + cx.assert_editor_state(&format!( + "«aˇ»bc\n«aˇ»bc{large_body_1} «aˇ»bc{large_body_2}\nef«aˇ»bc\n«aˇ»bc" + )); + cx.update_editor(|editor, _, cx| { + assert_eq!( + editor.scroll_position(cx), + initial_scroll_position, + "Scroll position should not change after selecting all matches" + ) + }); + + cx.simulate_keystroke("x"); + cx.update_editor(|editor, _, cx| { + assert_eq!( + editor.scroll_position(cx), + initial_scroll_position, + "Scroll position should not change after editing all matches" + ) + }); } #[gpui::test]