From 3a8d012d1a846272c2ba019176352bbd1befa6cf Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Wed, 13 May 2026 17:35:10 -0600 Subject: [PATCH] Fix macOS find query seeding (#56681) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #55619 ### Summary - Route `buffer_search::UseSelectionForFind` through `BufferSearchBar::deploy` instead of updating the query editor directly. - Add an explicit seed-query override to `deploy`, so the Cmd-E action can force `SeedQuerySetting::Always` while regular deploy callers continue to pass `None` and respect the user’s `seed_search_query_from_cursor` setting. - By going through `deploy`, Cmd-E now also runs the search path that keeps buffer-search navigation state in sync: - shows/initializes the search bar for the active searchable item - applies the seeded query via `search_suggested` - calls `search`, which updates the query editor, search options, active search query, search history, and macOS find pasteboard - refreshes `searchable_items_with_matches` and `active_match_index` - activates the current match after the search completes - This ensures the subsequent Cmd-G action has the expected active query, match list, search token, and active match index to select the next result. - Add a macOS-only end-to-end regression test using the default macOS keymap with `simulate_keystrokes("cmd-e")` and `simulate_keystrokes("cmd-g")`. ### Validation - `cargo test -p search test_cmd_e_then_cmd_g_uses_selection_for_find` - `cargo fmt --check --package search --package zed_actions` - `./script/check-keymaps` - `cargo check -p search` - `cargo check -p workspace` - `cargo check -p vim` Release Notes: - Fixed macOS Cmd-E/Cmd-G find behavior so Cmd-E seeds find from the cursor or selection and Cmd-G advances through the newly seeded matches. --- crates/debugger_tools/src/dap_log.rs | 9 +- crates/editor/src/items.rs | 9 +- crates/editor/src/split.rs | 6 +- crates/language_tools/src/lsp_log_view.rs | 8 +- .../src/markdown_preview_view.rs | 4 +- crates/search/src/buffer_search.rs | 215 +++++++++++++++--- crates/search/src/project_search.rs | 2 +- crates/terminal_view/src/terminal_view.rs | 6 +- crates/vim/src/normal/search.rs | 6 +- crates/workspace/src/searchable.rs | 19 +- crates/zed_actions/src/lib.rs | 2 +- 11 files changed, 223 insertions(+), 63 deletions(-) diff --git a/crates/debugger_tools/src/dap_log.rs b/crates/debugger_tools/src/dap_log.rs index 76d31bdd232..6faecff5e32 100644 --- a/crates/debugger_tools/src/dap_log.rs +++ b/crates/debugger_tools/src/dap_log.rs @@ -19,7 +19,7 @@ use project::{ debugger::{dap_store, session::Session}, search::SearchQuery, }; -use settings::Settings as _; +use settings::{SeedQuerySetting, Settings as _}; use std::{ borrow::Cow, collections::{BTreeMap, HashMap, VecDeque}, @@ -1031,12 +1031,13 @@ impl SearchableItem for DapLogView { fn query_suggestion( &mut self, - ignore_settings: bool, + seed_query_override: Option, window: &mut Window, cx: &mut Context, ) -> String { - self.editor - .update(cx, |e, cx| e.query_suggestion(ignore_settings, window, cx)) + self.editor.update(cx, |e, cx| { + e.query_suggestion(seed_query_override, window, cx) + }) } fn activate_match( diff --git a/crates/editor/src/items.rs b/crates/editor/src/items.rs index 9434da98973..5b46a735dca 100644 --- a/crates/editor/src/items.rs +++ b/crates/editor/src/items.rs @@ -1699,15 +1699,12 @@ impl SearchableItem for Editor { fn query_suggestion( &mut self, - ignore_settings: bool, + seed_query_override: Option, window: &mut Window, cx: &mut Context, ) -> String { - let setting = if ignore_settings { - SeedQuerySetting::Always - } else { - EditorSettings::get_global(cx).seed_search_query_from_cursor - }; + let setting = seed_query_override + .unwrap_or_else(|| EditorSettings::get_global(cx).seed_search_query_from_cursor); let snapshot = self.snapshot(window, cx); let selection = self.selections.newest_adjusted(&snapshot.display_snapshot); let buffer_snapshot = snapshot.buffer_snapshot(); diff --git a/crates/editor/src/split.rs b/crates/editor/src/split.rs index 39c450fb959..393e5971c56 100644 --- a/crates/editor/src/split.rs +++ b/crates/editor/src/split.rs @@ -18,7 +18,7 @@ use multi_buffer::{ }; use project::Project; use rope::Point; -use settings::{DiffViewStyle, Settings}; +use settings::{DiffViewStyle, SeedQuerySetting, Settings}; use text::{Bias, BufferId, OffsetRangeExt as _, Patch, ToPoint as _}; use ui::{ App, Context, InteractiveElement as _, IntoElement as _, ParentElement as _, Render, @@ -1940,12 +1940,12 @@ impl SearchableItem for SplittableEditor { fn query_suggestion( &mut self, - ignore_settings: bool, + seed_query_override: Option, window: &mut Window, cx: &mut Context, ) -> String { self.focused_editor().update(cx, |editor, cx| { - editor.query_suggestion(ignore_settings, window, cx) + editor.query_suggestion(seed_query_override, window, cx) }) } diff --git a/crates/language_tools/src/lsp_log_view.rs b/crates/language_tools/src/lsp_log_view.rs index a54f2961c79..159f06952ca 100644 --- a/crates/language_tools/src/lsp_log_view.rs +++ b/crates/language_tools/src/lsp_log_view.rs @@ -17,6 +17,7 @@ use project::{ search::SearchQuery, }; use proto::toggle_lsp_logs::LogType; +use settings::SeedQuerySetting; use std::{any::TypeId, borrow::Cow, sync::Arc}; use ui::{Checkbox, ContextMenu, PopoverMenu, ToggleState, prelude::*}; use util::ResultExt as _; @@ -824,12 +825,13 @@ impl SearchableItem for LspLogView { fn query_suggestion( &mut self, - ignore_settings: bool, + seed_query_override: Option, window: &mut Window, cx: &mut Context, ) -> String { - self.editor - .update(cx, |e, cx| e.query_suggestion(ignore_settings, window, cx)) + self.editor.update(cx, |e, cx| { + e.query_suggestion(seed_query_override, window, cx) + }) } fn activate_match( diff --git a/crates/markdown_preview/src/markdown_preview_view.rs b/crates/markdown_preview/src/markdown_preview_view.rs index 333a1b2a2b3..79749936d06 100644 --- a/crates/markdown_preview/src/markdown_preview_view.rs +++ b/crates/markdown_preview/src/markdown_preview_view.rs @@ -20,7 +20,7 @@ use markdown::{ }; use project::Project; use project::search::SearchQuery; -use settings::Settings; +use settings::{SeedQuerySetting, Settings}; use theme::{SystemAppearance, Theme, ThemeRegistry}; use theme_settings::ThemeSettings; use ui::{ContextMenu, WithScrollbar, prelude::*, right_click_menu}; @@ -1108,7 +1108,7 @@ impl SearchableItem for MarkdownPreviewView { fn query_suggestion( &mut self, - _ignore_settings: bool, + _seed_query_override: Option, _window: &mut Window, cx: &mut Context, ) -> String { diff --git a/crates/search/src/buffer_search.rs b/crates/search/src/buffer_search.rs index 30805264522..ea03a1a7f9f 100644 --- a/crates/search/src/buffer_search.rs +++ b/crates/search/src/buffer_search.rs @@ -31,7 +31,7 @@ use project::{ }; use fs::Fs; -use settings::{DiffViewStyle, Settings, update_settings_file}; +use settings::{DiffViewStyle, SeedQuerySetting, Settings, update_settings_file}; use std::{any::TypeId, sync::Arc}; use zed_actions::{ OpenSettingsAt, outline::ToggleOutline, workspace::CopyPath, workspace::CopyRelativePath, @@ -822,23 +822,23 @@ impl BufferSearchBar { // register deploy buffer search for both search bar states, since we want to focus into the search bar // when the deploy action is triggered in the buffer. registrar.register_handler(ForDeployed(|this, deploy, window, cx| { - this.deploy(deploy, window, cx); + this.deploy(deploy, None, window, cx); })); registrar.register_handler(ForDismissed(|this, deploy, window, cx| { - this.deploy(deploy, window, cx); + this.deploy(deploy, None, window, cx); })); registrar.register_handler(ForDeployed(|this, _: &DeployReplace, window, cx| { if this.supported_options(cx).find_in_results { cx.propagate(); } else { - this.deploy(&Deploy::replace(), window, cx); + this.deploy(&Deploy::replace(), None, window, cx); } })); registrar.register_handler(ForDismissed(|this, _: &DeployReplace, window, cx| { if this.supported_options(cx).find_in_results { cx.propagate(); } else { - this.deploy(&Deploy::replace(), window, cx); + this.deploy(&Deploy::replace(), None, window, cx); } })); registrar.register_handler(ForDeployed( @@ -978,7 +978,13 @@ impl BufferSearchBar { cx.notify(); } - pub fn deploy(&mut self, deploy: &Deploy, window: &mut Window, cx: &mut Context) -> bool { + pub fn deploy( + &mut self, + deploy: &Deploy, + seed_query_override: Option, + window: &mut Window, + cx: &mut Context, + ) -> bool { let filtered_search_range = if deploy.selection_search_enabled { Some(FilteredSearchRange::Default) } else { @@ -988,7 +994,7 @@ impl BufferSearchBar { if let Some(active_item) = self.active_searchable_item.as_mut() { active_item.toggle_filtered_search_ranges(filtered_search_range, window, cx); } - self.search_suggested(window, cx); + self.search_suggested(seed_query_override, window, cx); self.smartcase(window, cx); self.sync_select_next_case_sensitivity(cx); self.replace_enabled |= deploy.replace_enabled; @@ -1003,7 +1009,9 @@ impl BufferSearchBar { let mut handle = self.query_editor.focus_handle(cx); let mut select_query = true; - let has_seed_text = self.query_suggestion(false, window, cx).is_some(); + let has_seed_text = self + .query_suggestion(seed_query_override, window, cx) + .is_some(); if deploy.replace_enabled && has_seed_text { handle = self.replacement_editor.focus_handle(cx); select_query = false; @@ -1024,7 +1032,7 @@ impl BufferSearchBar { pub fn toggle(&mut self, action: &Deploy, window: &mut Window, cx: &mut Context) { if self.is_dismissed() { - self.deploy(action, window, cx); + self.deploy(action, None, window, cx); } else { self.dismiss(&Dismiss, window, cx); } @@ -1111,10 +1119,17 @@ impl BufferSearchBar { } } - pub fn search_suggested(&mut self, window: &mut Window, cx: &mut Context) { - let search = self.query_suggestion(false, window, cx).map(|suggestion| { - self.search(&suggestion, Some(self.default_options), true, window, cx) - }); + pub fn search_suggested( + &mut self, + seed_query_override: Option, + window: &mut Window, + cx: &mut Context, + ) { + let search = self + .query_suggestion(seed_query_override, window, cx) + .map(|suggestion| { + self.search(&suggestion, Some(self.default_options), true, window, cx) + }); #[cfg(target_os = "macos")] let search = search.or_else(|| { @@ -1166,13 +1181,15 @@ impl BufferSearchBar { pub fn query_suggestion( &mut self, - ignore_settings: bool, + seed_query_override: Option, window: &mut Window, cx: &mut Context, ) -> Option { self.active_searchable_item .as_ref() - .map(|searchable_item| searchable_item.query_suggestion(ignore_settings, window, cx)) + .map(|searchable_item| { + searchable_item.query_suggestion(seed_query_override, window, cx) + }) .filter(|suggestion| !suggestion.is_empty()) } @@ -1243,18 +1260,16 @@ impl BufferSearchBar { window: &mut Window, cx: &mut Context, ) { - let Some(search_text) = self.query_suggestion(true, window, cx) else { - return; - }; - self.query_editor.update(cx, |query_editor, cx| { - query_editor.buffer().update(cx, |query_buffer, cx| { - let len = query_buffer.len(cx); - query_buffer.edit([(MultiBufferOffset(0)..len, search_text)], None, cx); - }); - }); - #[cfg(target_os = "macos")] - self.update_find_pasteboard(cx); - cx.notify(); + self.deploy( + &Deploy { + focus: false, + replace_enabled: false, + selection_search_enabled: false, + }, + Some(SeedQuerySetting::Always), + window, + cx, + ); } pub fn focus_editor(&mut self, _: &FocusEditor, window: &mut Window, cx: &mut Context) { @@ -1945,9 +1960,13 @@ mod tests { use futures::stream::StreamExt as _; use gpui::{Hsla, TestAppContext, UpdateGlobal, VisualTestContext}; use language::{Buffer, Point}; + #[cfg(target_os = "macos")] + use project::Project; use settings::{SearchSettingsContent, SettingsStore}; use unindent::Unindent as _; use util_macros::perf; + #[cfg(target_os = "macos")] + use workspace::{AppState, MultiWorkspace, Workspace}; fn init_globals(cx: &mut TestAppContext) { cx.update(|cx| { @@ -2401,7 +2420,7 @@ mod tests { // search_suggested should restore default options search_bar.update_in(cx, |search_bar, window, cx| { - search_bar.search_suggested(window, cx); + search_bar.search_suggested(None, window, cx); assert_eq!(search_bar.search_options, SearchOptions::NONE) }); @@ -2432,7 +2451,7 @@ mod tests { // defaults should still include whole word search_bar.update_in(cx, |search_bar, window, cx| { - search_bar.search_suggested(window, cx); + search_bar.search_suggested(None, window, cx); assert_eq!( search_bar.search_options, SearchOptions::CASE_SENSITIVE | SearchOptions::WHOLE_WORD @@ -3255,6 +3274,7 @@ mod tests { replace_enabled: true, selection_search_enabled: false, }, + None, window, cx, ); @@ -3276,6 +3296,129 @@ mod tests { }); } + #[cfg(target_os = "macos")] + #[gpui::test] + async fn test_cmd_e_then_cmd_g_uses_selection_for_find(cx: &mut TestAppContext) { + init_globals(cx); + let app_state = cx.update(AppState::test); + let project = Project::test(app_state.fs.clone(), [], cx).await; + let buffer = cx.new(|cx| { + Buffer::local( + r#" + dad + cat + mom + dog + dog + cat + dad + mom + "# + .unindent(), + cx, + ) + }); + let multibuffer = cx.update(|cx| MultiBuffer::build_from_buffer(buffer, cx)); + let mut editor = None; + let mut search_bar = None; + + let window = cx.add_window(|window, cx| { + let default_key_bindings = settings::KeymapFile::load_asset_allow_partial_failure( + "keymaps/default-macos.json", + cx, + ) + .unwrap(); + cx.bind_keys(default_key_bindings); + let workspace = cx.new(|cx| Workspace::test_new(project.clone(), window, cx)); + let multi_workspace = MultiWorkspace::new(workspace.clone(), window, cx); + let buffer_search_bar = cx.new(|cx| BufferSearchBar::new(None, window, cx)); + workspace.update(cx, |workspace, cx| { + workspace.active_pane().update(cx, |pane, cx| { + pane.toolbar().update(cx, |toolbar, cx| { + toolbar.add_item(buffer_search_bar.clone(), window, cx); + }); + }); + }); + let editor_handle = cx.new(|cx| { + Editor::new( + editor::EditorMode::full(), + multibuffer.clone(), + Some(project.clone()), + window, + cx, + ) + }); + workspace.update(cx, |workspace, cx| { + workspace.add_item_to_center(Box::new(editor_handle.clone()), window, cx); + }); + window.focus(&editor_handle.focus_handle(cx), cx); + search_bar = Some(buffer_search_bar); + editor = Some(editor_handle); + multi_workspace + }); + let cx = VisualTestContext::from_window(*window, cx).into_mut(); + let editor = editor.unwrap(); + let search_bar = search_bar.unwrap(); + + editor.update_in(cx, |editor, window, cx| { + editor.change_selections(SelectionEffects::no_scroll(), window, cx, |selections| { + selections.select_display_ranges([ + DisplayPoint::new(DisplayRow(3), 1)..DisplayPoint::new(DisplayRow(3), 1) + ]); + }); + }); + + cx.simulate_keystrokes("cmd-e"); + + search_bar.read_with(cx, |search_bar, cx| { + assert_eq!(search_bar.query(cx), "dog"); + assert_eq!(search_bar.active_match_index, Some(0)); + }); + cx.read(|cx| { + assert_eq!( + cx.read_from_find_pasteboard().and_then(|item| item.text()), + Some("dog".to_string()) + ); + }); + + cx.simulate_keystrokes("cmd-g"); + assert_eq!( + editor.update(cx, |editor, cx| editor + .selections + .display_ranges(&editor.display_snapshot(cx))), + [DisplayPoint::new(DisplayRow(4), 0)..DisplayPoint::new(DisplayRow(4), 3)] + ); + + editor.update_in(cx, |editor, window, cx| { + editor.change_selections(SelectionEffects::no_scroll(), window, cx, |selections| { + selections.select_display_ranges([ + DisplayPoint::new(DisplayRow(1), 1)..DisplayPoint::new(DisplayRow(1), 1) + ]); + }); + }); + + cx.simulate_keystrokes("cmd-e"); + + search_bar.read_with(cx, |search_bar, cx| { + assert_eq!(search_bar.query(cx), "cat"); + assert_eq!(search_bar.active_match_index, Some(0)); + }); + cx.read(|cx| { + assert_eq!( + cx.read_from_find_pasteboard().and_then(|item| item.text()), + Some("cat".to_string()) + ); + }); + + cx.simulate_keystrokes("cmd-g"); + assert_eq!( + editor.update(cx, |editor, cx| editor + .selections + .display_ranges(&editor.display_snapshot(cx))), + [DisplayPoint::new(DisplayRow(5), 0)..DisplayPoint::new(DisplayRow(5), 3)] + ); + } + #[perf] #[gpui::test] async fn test_find_matches_in_selections_singleton_buffer_multiple_selections( @@ -3319,7 +3462,7 @@ mod tests { replace_enabled: false, selection_search_enabled: true, }; - search_bar.deploy(&deploy, window, cx); + search_bar.deploy(&deploy, None, window, cx); }); cx.run_until_parked(); @@ -3406,7 +3549,7 @@ mod tests { replace_enabled: false, selection_search_enabled: true, }; - search_bar.deploy(&deploy, window, cx); + search_bar.deploy(&deploy, None, window, cx); }); cx.run_until_parked(); @@ -3673,7 +3816,7 @@ mod tests { !search_bar.dismissed, "Search bar should be present and visible" ); - search_bar.deploy(&deploy, window, cx); + search_bar.deploy(&deploy, None, window, cx); assert_eq!( search_bar.search_options, SearchOptions::WHOLE_WORD, @@ -3681,7 +3824,7 @@ mod tests { ); search_bar.dismiss(&Dismiss, window, cx); - search_bar.deploy(&deploy, window, cx); + search_bar.deploy(&deploy, None, window, cx); assert_eq!( search_bar.search_options, SearchOptions::WHOLE_WORD, @@ -3720,14 +3863,14 @@ mod tests { "Should have no search options enabled by default" ); - search_bar.deploy(&deploy, window, cx); + search_bar.deploy(&deploy, None, window, cx); assert_eq!( search_bar.search_options, SearchOptions::REGEX | SearchOptions::WHOLE_WORD, "Toggling a non-dismissed search bar with custom options should not change the default options" ); search_bar.dismiss(&Dismiss, window, cx); - search_bar.deploy(&deploy, window, cx); + search_bar.deploy(&deploy, None, window, cx); assert_eq!( search_bar.configured_options, SearchOptions::CASE_SENSITIVE, @@ -3753,7 +3896,7 @@ mod tests { ); search_bar.update_in(cx, |search_bar, window, cx| { - search_bar.deploy(&deploy, window, cx); + search_bar.deploy(&deploy, None, window, cx); search_bar.dismiss(&Dismiss, window, cx); search_bar.show(window, cx); assert_eq!( diff --git a/crates/search/src/project_search.rs b/crates/search/src/project_search.rs index 1ca53632dba..9aff505f907 100644 --- a/crates/search/src/project_search.rs +++ b/crates/search/src/project_search.rs @@ -1222,7 +1222,7 @@ impl ProjectSearchView { } let editor = item.act_as::(cx)?; - let query = editor.query_suggestion(false, window, cx); + let query = editor.query_suggestion(None, window, cx); if query.is_empty() { None } else { Some(query) } }); diff --git a/crates/terminal_view/src/terminal_view.rs b/crates/terminal_view/src/terminal_view.rs index 37c4c165836..449e1c67d79 100644 --- a/crates/terminal_view/src/terminal_view.rs +++ b/crates/terminal_view/src/terminal_view.rs @@ -20,7 +20,9 @@ use persistence::TerminalDb; use project::{Project, ProjectEntryId, search::SearchQuery}; use schemars::JsonSchema; use serde::Deserialize; -use settings::{Settings, SettingsStore, TerminalBell, TerminalBlink, WorkingDirectory}; +use settings::{ + SeedQuerySetting, Settings, SettingsStore, TerminalBell, TerminalBlink, WorkingDirectory, +}; use std::{ any::Any, cmp, @@ -1878,7 +1880,7 @@ impl SearchableItem for TerminalView { /// Returns the selection content to pre-load into this search fn query_suggestion( &mut self, - _ignore_settings: bool, + _seed_query_override: Option, _window: &mut Window, cx: &mut Context, ) -> String { diff --git a/crates/vim/src/normal/search.rs b/crates/vim/src/normal/search.rs index 4fde2f786ce..1ebf51aa293 100644 --- a/crates/vim/src/normal/search.rs +++ b/crates/vim/src/normal/search.rs @@ -455,7 +455,11 @@ impl Vim { if !search_bar.show(window, cx) { return None; } - let Some(query) = search_bar.query_suggestion(true, window, cx) else { + let Some(query) = search_bar.query_suggestion( + Some(settings::SeedQuerySetting::Always), + window, + cx, + ) else { drop(search_bar.search("", None, false, window, cx)); return None; }; diff --git a/crates/workspace/src/searchable.rs b/crates/workspace/src/searchable.rs index 1e689986833..78263f0f9d3 100644 --- a/crates/workspace/src/searchable.rs +++ b/crates/workspace/src/searchable.rs @@ -6,6 +6,7 @@ use gpui::{ Window, }; use project::search::SearchQuery; +use settings::SeedQuerySetting; use crate::{ ItemHandle, @@ -118,7 +119,7 @@ pub trait SearchableItem: Item + EventEmitter { ); fn query_suggestion( &mut self, - ignore_settings: bool, + seed_query_override: Option, window: &mut Window, cx: &mut Context, ) -> String; @@ -226,7 +227,12 @@ pub trait SearchableItemHandle: ItemHandle { window: &mut Window, cx: &mut App, ); - fn query_suggestion(&self, ignore_settings: bool, window: &mut Window, cx: &mut App) -> String; + fn query_suggestion( + &self, + seed_query_override: Option, + window: &mut Window, + cx: &mut App, + ) -> String; fn activate_match( &self, index: usize, @@ -340,9 +346,14 @@ impl SearchableItemHandle for Entity { this.update_matches(matches.as_slice(), active_match_index, token, window, cx) }); } - fn query_suggestion(&self, ignore_settings: bool, window: &mut Window, cx: &mut App) -> String { + fn query_suggestion( + &self, + seed_query_override: Option, + window: &mut Window, + cx: &mut App, + ) -> String { self.update(cx, |this, cx| { - this.query_suggestion(ignore_settings, window, cx) + this.query_suggestion(seed_query_override, window, cx) }) } fn activate_match( diff --git a/crates/zed_actions/src/lib.rs b/crates/zed_actions/src/lib.rs index f09fa27ddf2..6f35c4e3535 100644 --- a/crates/zed_actions/src/lib.rs +++ b/crates/zed_actions/src/lib.rs @@ -468,7 +468,7 @@ pub mod buffer_search { Dismiss, /// Focuses back on the editor. FocusEditor, - /// Sets the search query to the current selection without opening the search bar or running a search. + /// Sets the search query from the selection or word under cursor. UseSelectionForFind, ] );