From 609e915b10833210b3d3905e636dd36c394de46a Mon Sep 17 00:00:00 2001 From: lex00 <121451605+lex00@users.noreply.github.com> Date: Fri, 30 Jan 2026 14:59:38 -0700 Subject: [PATCH] vim: Restore cursor position when dismissing buffer search (#47732) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #8048 ## Summary In vim mode, pressing Escape to dismiss the buffer search now correctly restores the cursor to its original position, rather than leaving it at the first match. ## Problem When using vim's `/` command to search: 1. User positions cursor at line X 2. User presses `/` to open search, types a query 3. Matches are highlighted, cursor may visually jump to first match 4. User presses Escape to dismiss without navigating 5. **Bug:** Cursor ends up at first match instead of line X This breaks vim parity where Escape should cancel the search and restore cursor position. ## Solution The fix leverages the `focused()` callback in `vim.rs`, which is called when the editor regains focus after the search bar is dismissed. **Key insight:** When search starts via `/`, the cursor position is saved in `SearchState.prior_selections`. When search is *submitted* with Enter, `search_submit()` drains these selections. But when search is *dismissed* with Escape, they remain. So in `focused()`, if: - `prior_selections` is non-empty, AND - The search bar's `is_dismissed()` returns true ...then we know the user dismissed the search (Escape) rather than submitted it (Enter), and we restore the cursor. ## Why not handle `buffer_search::Dismiss` directly? The initial approach tried to register a vim handler for the `Dismiss` action. This didn't work because when Escape is pressed, the search bar (which has focus) handles the `Cancel` action internally and calls its `dismiss()` method directly—it doesn't dispatch `Dismiss` through the action system. The vim handler registered on the editor was never invoked. ## Test Plan - Added `test_search_dismiss_restores_cursor` — verifies cursor restoration when search is dismissed - Added `test_search_dismiss_restores_cursor_no_matches` — verifies behavior when query has no matches - All 455 vim tests pass - Manual testing confirms fix works with both `/` and `cmd-f` ## Release Notes - Fixed vim mode: cursor now returns to original position when dismissing buffer search with Escape (#8048) --------- Co-authored-by: Claude Opus 4.5 --- crates/search/src/buffer_search.rs | 2 + crates/vim/src/helix.rs | 1 + crates/vim/src/normal/search.rs | 180 +++++++++++++++++++++++------ crates/vim/src/state.rs | 3 +- crates/vim/src/vim.rs | 18 +++ 5 files changed, 168 insertions(+), 36 deletions(-) diff --git a/crates/search/src/buffer_search.rs b/crates/search/src/buffer_search.rs index c8b860beb29dd959de06a3075dec6b7d3eebac06..888ba58c83a0f545e7ec43a79511e2b2347e7e62 100644 --- a/crates/search/src/buffer_search.rs +++ b/crates/search/src/buffer_search.rs @@ -94,6 +94,7 @@ impl Deploy { pub enum Event { UpdateLocation, + Dismissed, } pub fn init(cx: &mut App) { @@ -826,6 +827,7 @@ impl BufferSearchBar { pub fn dismiss(&mut self, _: &Dismiss, window: &mut Window, cx: &mut Context) { self.dismissed = true; + cx.emit(Event::Dismissed); self.query_error = None; self.sync_select_next_case_sensitivity(cx); diff --git a/crates/vim/src/helix.rs b/crates/vim/src/helix.rs index 4602862e0514db64870e5a7fb41c57f073cd4d98..cfcf874e0325023107bf2dc779029738fe7b6bca 100644 --- a/crates/vim/src/helix.rs +++ b/crates/vim/src/helix.rs @@ -583,6 +583,7 @@ impl Vim { prior_operator: self.operator_stack.last().cloned(), prior_mode: self.mode, helix_select: true, + _dismiss_subscription: None, } }); } diff --git a/crates/vim/src/normal/search.rs b/crates/vim/src/normal/search.rs index 4828ae9560065c4824ea5cd38813c459d91f8882..c11784d163e18451129656aa92d23dba568bd723 100644 --- a/crates/vim/src/normal/search.rs +++ b/crates/vim/src/normal/search.rs @@ -225,45 +225,71 @@ impl Vim { let count = Vim::take_count(cx).unwrap_or(1); Vim::take_forced_motion(cx); let prior_selections = self.editor_selections(window, cx); - pane.update(cx, |pane, cx| { - if let Some(search_bar) = pane.toolbar().read(cx).item_of_type::() { - search_bar.update(cx, |search_bar, cx| { - if !search_bar.show(window, cx) { - return; - } - search_bar.select_query(window, cx); - cx.focus_self(window); + let Some(search_bar) = pane + .read(cx) + .toolbar() + .read(cx) + .item_of_type::() + else { + return; + }; - search_bar.set_replacement(None, cx); - let mut options = SearchOptions::NONE; - if action.regex { - options |= SearchOptions::REGEX; - } - if action.backwards { - options |= SearchOptions::BACKWARDS; - } - if EditorSettings::get_global(cx).search.case_sensitive { - options |= SearchOptions::CASE_SENSITIVE; - } - search_bar.set_search_options(options, cx); - let prior_mode = if self.temp_mode { - Mode::Insert - } else { - self.mode - }; + let shown = search_bar.update(cx, |search_bar, cx| { + if !search_bar.show(window, cx) { + return false; + } - self.search = SearchState { - direction, - count, - prior_selections, - prior_operator: self.operator_stack.last().cloned(), - prior_mode, - helix_select: false, - } - }); + search_bar.select_query(window, cx); + cx.focus_self(window); + + search_bar.set_replacement(None, cx); + let mut options = SearchOptions::NONE; + if action.regex { + options |= SearchOptions::REGEX; } - }) + if action.backwards { + options |= SearchOptions::BACKWARDS; + } + if EditorSettings::get_global(cx).search.case_sensitive { + options |= SearchOptions::CASE_SENSITIVE; + } + search_bar.set_search_options(options, cx); + true + }); + + if !shown { + return; + } + + let subscription = cx.subscribe_in(&search_bar, window, |vim, _, event, window, cx| { + if let buffer_search::Event::Dismissed = event { + if !vim.search.prior_selections.is_empty() { + let prior_selections: Vec<_> = vim.search.prior_selections.drain(..).collect(); + vim.update_editor(cx, |_, editor, cx| { + editor.change_selections(Default::default(), window, cx, |s| { + s.select_ranges(prior_selections); + }); + }); + } + } + }); + + let prior_mode = if self.temp_mode { + Mode::Insert + } else { + self.mode + }; + + self.search = SearchState { + direction, + count, + prior_selections, + prior_operator: self.operator_stack.last().cloned(), + prior_mode, + helix_select: false, + _dismiss_subscription: Some(subscription), + } } // hook into the existing to clear out any vim search state on cmd+f or edit -> find. @@ -1295,4 +1321,88 @@ mod test { " }); } + + #[gpui::test] + async fn test_search_dismiss_restores_cursor(cx: &mut gpui::TestAppContext) { + let mut cx = VimTestContext::new(cx, true).await; + cx.set_state("ˇhello world\nfoo bar\nhello again\n", Mode::Normal); + + // Move cursor to line 2 + cx.simulate_keystrokes("j"); + cx.run_until_parked(); + cx.assert_state("hello world\nˇfoo bar\nhello again\n", Mode::Normal); + + // Open search + cx.simulate_keystrokes("/"); + cx.run_until_parked(); + + // Dismiss search with Escape - cursor should return to line 2 + cx.simulate_keystrokes("escape"); + cx.run_until_parked(); + // Cursor should be restored to line 2 where it was when search was opened + cx.assert_state("hello world\nˇfoo bar\nhello again\n", Mode::Normal); + } + + #[gpui::test] + async fn test_search_dismiss_restores_cursor_no_matches(cx: &mut gpui::TestAppContext) { + let mut cx = VimTestContext::new(cx, true).await; + cx.set_state("ˇapple\nbanana\ncherry\n", Mode::Normal); + + // Move cursor to line 2 + cx.simulate_keystrokes("j"); + cx.run_until_parked(); + cx.assert_state("apple\nˇbanana\ncherry\n", Mode::Normal); + + // Open search and type query for something that doesn't exist + cx.simulate_keystrokes("/ n o n e x i s t e n t"); + cx.run_until_parked(); + + // Dismiss search with Escape - cursor should still be at original position + cx.simulate_keystrokes("escape"); + cx.run_until_parked(); + cx.assert_state("apple\nˇbanana\ncherry\n", Mode::Normal); + } + + #[gpui::test] + async fn test_search_dismiss_after_editor_focus_does_not_restore( + cx: &mut gpui::TestAppContext, + ) { + let mut cx = VimTestContext::new(cx, true).await; + cx.set_state("ˇhello world\nfoo bar\nhello again\n", Mode::Normal); + + // Move cursor to line 2 + cx.simulate_keystrokes("j"); + cx.run_until_parked(); + cx.assert_state("hello world\nˇfoo bar\nhello again\n", Mode::Normal); + + // Open search and type a query that matches line 3 + cx.simulate_keystrokes("/ a g a i n"); + cx.run_until_parked(); + + // Simulate the editor gaining focus while search is still open + // This represents the user clicking in the editor + cx.update_editor(|_, window, cx| cx.focus_self(window)); + cx.run_until_parked(); + + // Now dismiss the search bar directly + cx.workspace(|workspace, window, cx| { + let pane = workspace.active_pane().read(cx); + if let Some(search_bar) = pane + .toolbar() + .read(cx) + .item_of_type::() + { + search_bar.update(cx, |bar, cx| { + bar.dismiss(&search::buffer_search::Dismiss, window, cx) + }); + } + }); + cx.run_until_parked(); + + // Cursor should NOT be restored to line 2 (row 1) where search was opened. + // Since the user "clicked" in the editor (by focusing it), prior_selections + // was cleared, so dismiss should not restore the cursor. + // The cursor should be at the match location on line 3 (row 2). + cx.assert_state("hello world\nfoo bar\nhello ˇagain\n", Mode::Normal); + } } diff --git a/crates/vim/src/state.rs b/crates/vim/src/state.rs index f4c3af7131d6e76be9046f90ff425282ce03e97e..1075a1144355083bd410b3aee4d015031f946a4e 100644 --- a/crates/vim/src/state.rs +++ b/crates/vim/src/state.rs @@ -995,7 +995,7 @@ impl Clone for ReplayableAction { } } -#[derive(Clone, Default, Debug)] +#[derive(Default, Debug)] pub struct SearchState { pub direction: Direction, pub count: usize, @@ -1004,6 +1004,7 @@ pub struct SearchState { pub prior_operator: Option, pub prior_mode: Mode, pub helix_select: bool, + pub _dismiss_subscription: Option, } impl Operator { diff --git a/crates/vim/src/vim.rs b/crates/vim/src/vim.rs index 2ede08172f322f41c16369714e28ca4c894b53ef..8e21b2b7a795a20947d5697c034a2bb6ee425f55 100644 --- a/crates/vim/src/vim.rs +++ b/crates/vim/src/vim.rs @@ -42,6 +42,7 @@ use multi_buffer::ToPoint as _; use normal::search::SearchSubmit; use object::Object; use schemars::JsonSchema; +use search::BufferSearchBar; use serde::Deserialize; use settings::RegisterSetting; pub use settings::{ @@ -1427,6 +1428,23 @@ impl Vim { } fn focused(&mut self, preserve_selection: bool, window: &mut Window, cx: &mut Context) { + // If editor gains focus while search bar is still open (not dismissed), + // the user has explicitly navigated away - clear prior_selections so we + // don't restore to the old position if they later dismiss the search. + if !self.search.prior_selections.is_empty() { + if let Some(pane) = self.pane(window, cx) { + let search_still_open = pane + .read(cx) + .toolbar() + .read(cx) + .item_of_type::() + .is_some_and(|bar| !bar.read(cx).is_dismissed()); + if search_still_open { + self.search.prior_selections.clear(); + } + } + } + let Some(editor) = self.editor() else { return; };