From 0446ad229437b180d03b3442c53d9027c4609f61 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Tue, 15 Sep 2026 09:58:45 +0000 Subject: [PATCH] Refine recent commands' UX (#64181) Follow-up to https://github.com/zed-industries/zed/pull/63388 https://github.com/user-attachments/assets/2a855451-141b-4030-8475-92f23c661d51 Release Notes: - Improved recent commands' UX --- assets/keymaps/default-linux.json | 6 + assets/keymaps/default-macos.json | 7 + assets/keymaps/default-windows.json | 7 + assets/settings/default.json | 5 + crates/command_palette/src/command_palette.rs | 410 ++++++++++++++++-- .../src/command_palette_settings.rs | 18 + crates/command_palette/src/persistence.rs | 33 ++ crates/picker/src/picker.rs | 77 +++- crates/settings/src/vscode_import.rs | 25 ++ .../settings_content/src/settings_content.rs | 14 +- crates/settings_ui/src/page_data.rs | 36 +- docs/src/reference/all-settings.md | 18 + 12 files changed, 615 insertions(+), 41 deletions(-) create mode 100644 crates/command_palette/src/command_palette_settings.rs diff --git a/assets/keymaps/default-linux.json b/assets/keymaps/default-linux.json index 763bf84574..62f12b87b4 100644 --- a/assets/keymaps/default-linux.json +++ b/assets/keymaps/default-linux.json @@ -1250,6 +1250,12 @@ "ctrl-shift-i": "search::ToggleIncludeIgnored", }, }, + { + "context": "CommandPalette || (CommandPalette > Picker > Editor)", + "bindings": { + "shift-backspace": "command_palette::RemoveSelected", + }, + }, { "context": "RecentProjects || (RecentProjects > Picker > Editor)", "bindings": { diff --git a/assets/keymaps/default-macos.json b/assets/keymaps/default-macos.json index 15bf7cabe7..f436e50eee 100644 --- a/assets/keymaps/default-macos.json +++ b/assets/keymaps/default-macos.json @@ -1304,6 +1304,13 @@ "cmd-shift-i": "search::ToggleIncludeIgnored", }, }, + { + "context": "CommandPalette || (CommandPalette > Picker > Editor)", + "use_key_equivalents": true, + "bindings": { + "shift-backspace": "command_palette::RemoveSelected", + }, + }, { "context": "RecentProjects || (RecentProjects > Picker > Editor)", "use_key_equivalents": true, diff --git a/assets/keymaps/default-windows.json b/assets/keymaps/default-windows.json index b523118161..fff87c886b 100644 --- a/assets/keymaps/default-windows.json +++ b/assets/keymaps/default-windows.json @@ -1261,6 +1261,13 @@ "ctrl-shift-i": "search::ToggleIncludeIgnored", }, }, + { + "context": "CommandPalette || (CommandPalette > Picker > Editor)", + "use_key_equivalents": true, + "bindings": { + "shift-backspace": "command_palette::RemoveSelected", + }, + }, { "context": "RecentProjects || (RecentProjects > Picker > Editor)", "use_key_equivalents": true, diff --git a/assets/settings/default.json b/assets/settings/default.json index fd52963231..75afc59d01 100644 --- a/assets/settings/default.json +++ b/assets/settings/default.json @@ -1506,6 +1506,11 @@ // Default: medium "modal_max_width": "medium", }, + // Settings related to the command palette. + "command_palette": { + // Whether to use command history ranking for sorting in the command palette. + "use_command_history": true, + }, // Settings related to the file finder. "file_finder": { // Whether to show file icons in the file finder. diff --git a/crates/command_palette/src/command_palette.rs b/crates/command_palette/src/command_palette.rs index 2b0e865f95..4d4117755d 100644 --- a/crates/command_palette/src/command_palette.rs +++ b/crates/command_palette/src/command_palette.rs @@ -1,3 +1,4 @@ +mod command_palette_settings; mod persistence; use std::{ @@ -7,28 +8,35 @@ use std::{ time::Duration, }; +use anyhow::Context as _; use client::parse_zed_link; use command_palette_hooks::{ CommandInterceptItem, CommandInterceptResult, CommandPaletteFilter, GlobalCommandPaletteInterceptor, }; +use command_palette_settings::CommandPaletteSettings; use fuzzy_nucleo::{StringMatch, StringMatchCandidate}; use gpui::{ - Action, App, Context, DismissEvent, Entity, EventEmitter, FocusHandle, Focusable, - ParentElement, Render, Styled, Task, TaskExt, WeakEntity, Window, + Action, App, Context, DismissEvent, Entity, EventEmitter, FocusHandle, Focusable, MouseButton, + ParentElement, Render, Styled, Task, TaskExt, WeakEntity, Window, actions, }; use persistence::CommandPaletteDB; use picker::Direction; use picker::{Picker, PickerDelegate}; use postage::{sink::Sink, stream::Stream}; use settings::Settings; -use ui::{HighlightedLabel, KeyBinding, ListItem, ListItemSpacing, prelude::*}; +use ui::{ + ButtonLike, HighlightedLabel, KeyBinding, ListItem, ListItemSpacing, Tooltip, prelude::*, +}; use util::ResultExt; use workspace::{ModalView, Workspace, WorkspaceSettings}; use zed_actions::{OpenZedUrl, command_palette::Toggle}; +actions!(command_palette, [RemoveSelected]); + pub fn init(cx: &mut App) { + CommandPaletteSettings::register(cx); command_palette_hooks::init(cx); cx.observe_new(CommandPalette::register).detach(); } @@ -120,8 +128,9 @@ impl CommandPalette { } Some(Command { - name: humanize_action_name(action.name()), + name: SharedString::from(humanize_action_name(action.name())), action, + usage: None, }) }) .collect(); @@ -136,6 +145,7 @@ impl CommandPalette { let picker = cx.new(|cx| { // One-shot action; there's nothing to reopen. let picker = Picker::uniform_list(delegate, window, cx) + .initial_width(rems(38.0)) .reopenable(false, cx) .show_scrollbar(true); picker.set_query(query, window, cx); @@ -148,6 +158,22 @@ impl CommandPalette { self.picker .update(cx, |picker, cx| picker.set_query(query, window, cx)) } + + fn remove_selected(&mut self, _: &RemoveSelected, window: &mut Window, cx: &mut Context) { + self.picker.update(cx, |picker, cx| { + let delegate = &mut picker.delegate; + let Some(command_name) = delegate + .selected_command() + .filter(|command| command.usage.is_some()) + .map(|command| command.name.clone()) + else { + return; + }; + delegate + .remove_command_history(command_name, window, cx) + .detach(); + }); + } } impl EventEmitter for CommandPalette {} @@ -159,9 +185,10 @@ impl Focusable for CommandPalette { } impl Render for CommandPalette { - fn render(&mut self, _window: &mut Window, _: &mut Context) -> impl IntoElement { + fn render(&mut self, _window: &mut Window, cx: &mut Context) -> impl IntoElement { v_flex() .key_context("CommandPalette") + .on_action(cx.listener(Self::remove_selected)) .child(self.picker.clone()) } } @@ -183,8 +210,9 @@ pub struct CommandPaletteDelegate { } struct Command { - name: String, + name: SharedString, action: Box, + usage: Option, } #[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] @@ -283,6 +311,7 @@ impl Clone for Command { Self { name: self.name.clone(), action: self.action.boxed_clone(), + usage: self.usage, } } } @@ -298,13 +327,13 @@ impl CommandPaletteDelegate { command_palette, workspace, all_commands: commands.clone(), - matches: vec![], + matches: Vec::new(), commands, selected_ix: 0, previous_focus_handle, latest_query: String::new(), updating_matches: None, - query_history: Default::default(), + query_history: QueryHistory::default(), } } @@ -333,13 +362,15 @@ impl CommandPaletteDelegate { { matches.remove(idx); } + let string = SharedString::from(string); commands.push(Command { name: string.clone(), action, + usage: None, }); new_matches.push(StringMatch { candidate_id: commands.len() - 1, - string: string.into(), + string, positions, score: 0.0, }) @@ -359,13 +390,17 @@ impl CommandPaletteDelegate { /// Hit count for each command in the palette. /// We only account for commands triggered directly via command palette and not by e.g. keystrokes because /// if a user already knows a keystroke for a command, they are unlikely to use a command palette to look for it. - fn command_usage(&self, cx: &App) -> HashMap { + fn command_usage(&self, cx: &App) -> HashMap { + if !CommandPaletteSettings::get_global(cx).use_command_history { + return HashMap::new(); + } + if let Ok(commands) = CommandPaletteDB::global(cx).list_commands_used() { commands .into_iter() .map(|command| { ( - command.command_name, + SharedString::from(command.command_name), CommandUsage { last_invoked: command.last_invoked.unix_timestamp(), invocations: command.invocations, @@ -378,6 +413,86 @@ impl CommandPaletteDelegate { } } + fn remove_command_history( + &mut self, + command_name: SharedString, + window: &mut Window, + cx: &mut Context>, + ) -> Task<()> { + let db = CommandPaletteDB::global(cx); + cx.spawn_in(window, async move |picker, cx| { + if db + .delete_command_history(command_name.to_string()) + .await + .with_context(|| format!("removing {command_name:?} from command history")) + .log_err() + .is_none() + { + return; + } + picker + .update_in(cx, |picker, window, cx| { + picker.delegate.query_history = QueryHistory::default(); + picker.refresh(window, cx); + }) + .ok(); + }) + } + + fn render_history_button( + ix: usize, + command_name: SharedString, + cx: &mut Context>, + ) -> impl IntoElement { + h_flex() + .id(("command-history", ix)) + .debug_selector(move || format!("command-history-{ix}")) + .group("command-history-button") + .on_mouse_down(MouseButton::Left, |_, window, cx| { + cx.stop_propagation(); + window.prevent_default(); + }) + .on_mouse_up(MouseButton::Right, |_, window, cx| { + cx.stop_propagation(); + window.prevent_default(); + }) + .child( + ButtonLike::new(("remove-command-history", ix)) + .aria_label("Remove from Command History") + .tooltip(Tooltip::for_action_title( + "Remove from Command History", + &RemoveSelected, + )) + .child( + h_flex() + .relative() + .child( + h_flex() + .group_hover("command-history-button", |this| this.invisible()) + .child( + Icon::new(IconName::HistoryRerun) + .color(Color::Muted) + .size(IconSize::Small), + ), + ) + .child( + h_flex() + .absolute() + .inset_0() + .visible_on_hover("command-history-button") + .child(Icon::new(IconName::Close).size(IconSize::Small)), + ), + ) + .on_click(cx.listener(move |picker, _, window, cx| { + window.prevent_default(); + picker + .delegate + .remove_command_history(command_name.clone(), window, cx) + .detach(); + })), + ) + } + fn selected_command(&self) -> Option<&Command> { if self.matches.is_empty() { return None; @@ -489,12 +604,10 @@ impl PickerDelegate for CommandPaletteDelegate { let query = normalize_action_query(query_str); let query_for_link = query_str.to_string(); async move { - commands.sort_by_key(|action| { - ( - Reverse(command_usage.get(&action.name).copied()), - action.name.clone(), - ) - }); + for command in &mut commands { + command.usage = command_usage.get(&command.name).copied(); + } + commands.sort_by_key(|command| (Reverse(command.usage), command.name.clone())); let candidates = commands .iter() @@ -515,7 +628,7 @@ impl PickerDelegate for CommandPaletteDelegate { let used_commands = commands .iter() - .take_while(|command| command_usage.contains_key(&command.name)) + .take_while(|command| command.usage.is_some()) .count(); matches.sort_by_key(|string_match| { if string_match.candidate_id < used_commands { @@ -668,15 +781,31 @@ impl PickerDelegate for CommandPaletteDelegate { .w_full() .py_px() .justify_between() - .child(HighlightedLabel::new( - command.name.clone(), - matching_command.positions.clone(), - )) - .child(KeyBinding::for_action_in( - &*command.action, - &self.previous_focus_handle, - cx, - )), + .gap_2() + .child( + HighlightedLabel::new( + command.name.clone(), + matching_command.positions.clone(), + ) + .truncate(), + ) + .child( + h_flex() + .flex_shrink_0() + .gap_2() + .child(KeyBinding::for_action_in( + &*command.action, + &self.previous_focus_handle, + cx, + )) + .when(command.usage.is_some(), |this| { + this.child(Self::render_history_button( + ix, + command.name.clone(), + cx, + )) + }), + ), ), ) } @@ -909,7 +1038,7 @@ mod tests { editor.update(cx, |editor, cx| window.focus(&editor.focus_handle(cx), cx)) }); - cx.simulate_keystrokes("cmd-shift-p"); + cx.dispatch_action(Toggle); let palette = workspace.update(cx, |workspace, cx| { workspace @@ -933,13 +1062,75 @@ mod tests { assert_eq!(palette.delegate.matches[0].string, "editor: backspace"); }); - cx.simulate_keystrokes("enter"); + cx.dispatch_action(menu::Confirm); workspace.update(cx, |workspace, cx| { assert!(workspace.active_modal::(cx).is_none()); assert_eq!(editor.read(cx).text(cx), "ab") }); + cx.dispatch_action(Toggle); + let history_button = cx.debug_bounds("command-history-0").unwrap(); + cx.simulate_mouse_move(history_button.center(), None, gpui::Modifiers::default()); + cx.simulate_mouse_down( + history_button.center(), + MouseButton::Right, + gpui::Modifiers::default(), + ); + cx.simulate_mouse_up( + history_button.center(), + MouseButton::Right, + gpui::Modifiers::default(), + ); + cx.simulate_mouse_down( + history_button.center(), + MouseButton::Left, + gpui::Modifiers::default(), + ); + let label_position = history_button.center() - gpui::point(px(100.0), px(0.0)); + cx.simulate_mouse_move( + label_position, + MouseButton::Left, + gpui::Modifiers::default(), + ); + cx.simulate_mouse_up( + label_position, + MouseButton::Left, + gpui::Modifiers::default(), + ); + workspace.read_with(cx, |workspace, cx| { + assert!(workspace.active_modal::(cx).is_some()); + assert_eq!(editor.read(cx).text(cx), "ab"); + }); + assert_eq!( + db.get_command_usage("editor: backspace") + .unwrap() + .unwrap() + .invocations, + 1 + ); + + cx.simulate_mouse_move(history_button.center(), None, gpui::Modifiers::default()); + cx.simulate_click(history_button.center(), gpui::Modifiers::default()); + cx.run_until_parked(); + + assert_eq!(db.get_command_usage("editor: backspace").unwrap(), None); + workspace.update_in(cx, |workspace, window, cx| { + assert_eq!(editor.read(cx).text(cx), "ab"); + let palette = workspace.active_modal::(cx).unwrap(); + let picker = palette.read(cx).picker.read(cx); + assert_eq!( + picker + .delegate + .matches + .iter() + .filter(|matching_command| matching_command.string == "editor: backspace") + .count(), + 1, + ); + workspace.hide_modal(window, cx); + }); + // Add namespace filter, and redeploy the palette cx.update(|_window, cx| { CommandPaletteFilter::update_global(cx, |filter, _| { @@ -947,7 +1138,7 @@ mod tests { }); }); - cx.simulate_keystrokes("cmd-shift-p"); + cx.dispatch_action(Toggle); cx.simulate_input("bcksp"); let palette = workspace.update(cx, |workspace, cx| { @@ -1029,8 +1220,8 @@ mod tests { cx.add_window_view(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx)); let workspace = multi_workspace.read_with(cx, |mw, _| mw.workspace().clone()); - cx.simulate_keystrokes("cmd-n"); - cx.simulate_keystrokes("cmd-shift-p"); + cx.dispatch_action(workspace::NewFile); + cx.dispatch_action(Toggle); let palette = workspace.update(cx, |workspace, cx| { workspace.active_modal::(cx).unwrap() @@ -1042,6 +1233,14 @@ mod tests { }); cx.run_until_parked(); + let unranked_names = picker.read_with(cx, |picker, _| { + picker + .delegate + .matches + .iter() + .map(|matching_command| matching_command.string.to_string()) + .collect::>() + }); let (worst_match, second_worst_match, match_count) = picker.read_with(cx, |picker, _| { let matches = &picker.delegate.matches; ( @@ -1081,6 +1280,150 @@ mod tests { assert_eq!(names[1], second_worst_match); assert_eq!(names.len(), match_count); }); + + picker.update_in(cx, |picker, window, cx| { + CommandPaletteSettings::override_global( + CommandPaletteSettings { + use_command_history: false, + }, + cx, + ); + picker.refresh(window, cx); + }); + cx.run_until_parked(); + cx.dispatch_action(RemoveSelected); + picker.read_with(cx, |picker, _| { + assert_eq!( + picker + .delegate + .matches + .iter() + .map(|matching_command| matching_command.string.to_string()) + .collect::>(), + unranked_names + ); + assert!( + picker + .delegate + .commands + .iter() + .all(|command| command.usage.is_none()) + ); + }); + assert_eq!(db.list_commands_used().unwrap().len(), 2); + + picker.update_in(cx, |picker, window, cx| { + CommandPaletteSettings::override_global( + CommandPaletteSettings { + use_command_history: true, + }, + cx, + ); + picker.refresh(window, cx); + }); + cx.run_until_parked(); + cx.dispatch_action(RemoveSelected); + cx.run_until_parked(); + picker.read_with(cx, |picker, _| { + let expected = std::iter::once(second_worst_match.clone()) + .chain( + unranked_names + .iter() + .filter(|name| *name != &second_worst_match) + .cloned(), + ) + .collect::>(); + assert_eq!( + picker + .delegate + .matches + .iter() + .map(|matching_command| matching_command.string.to_string()) + .collect::>(), + expected + ); + }); + + let remaining_usage = db.list_commands_used().unwrap(); + picker.update_in(cx, |picker, window, cx| { + picker.delegate.set_selected_index(1, window, cx); + }); + cx.dispatch_action(RemoveSelected); + assert_eq!(db.list_commands_used().unwrap(), remaining_usage); + } + + #[gpui::test] + async fn test_intercepted_commands_are_not_history(cx: &mut TestAppContext) { + let app_state = init_test(cx); + let project = Project::test(app_state.fs.clone(), [], cx).await; + let (multi_workspace, cx) = + cx.add_window_view(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx)); + let workspace = multi_workspace.read_with(cx, |workspace, _| workspace.workspace().clone()); + let picker = open_palette_with_history(&workspace, &[], cx); + + for (exclusive, expected) in [ + ( + false, + vec![("intercepted", false), ("older", true), ("unused", false)], + ), + (true, vec![("intercepted", false)]), + ] { + picker.update(cx, |picker, cx| { + let commands = [ + ("recent", Toggle.boxed_clone()), + ("older", menu::Confirm.boxed_clone()), + ("unused", menu::Cancel.boxed_clone()), + ] + .into_iter() + .enumerate() + .map(|(index, (name, action))| Command { + name: SharedString::from(name), + action, + usage: (index < 2).then_some(CommandUsage { + last_invoked: 0, + invocations: 1, + }), + }) + .collect::>(); + let matches = commands + .iter() + .enumerate() + .map(|(candidate_id, command)| StringMatch { + candidate_id, + string: command.name.clone(), + positions: Vec::new(), + score: 0.0, + }) + .collect::>(); + let intercept_result = CommandInterceptResult { + results: vec![CommandInterceptItem { + action: Toggle.boxed_clone(), + string: "intercepted".to_string(), + positions: Vec::new(), + }], + exclusive, + }; + picker.delegate.matches_updated( + String::new(), + commands, + matches, + intercept_result, + cx, + ); + assert_eq!( + picker + .delegate + .matches + .iter() + .map(|matching_command| { + let command = &picker.delegate.commands[matching_command.candidate_id]; + (command.name.as_str(), command.usage.is_some()) + }) + .collect::>(), + expected, + ); + }); + } } #[gpui::test] @@ -1103,6 +1446,7 @@ mod tests { cx.simulate_input("definitely-no-command-should-match-this"); cx.background_executor.run_until_parked(); + cx.dispatch_action(RemoveSelected); picker.read_with(cx, |picker, _cx| { assert!(picker.delegate.matches.is_empty()); @@ -1240,7 +1584,7 @@ mod tests { history: &[&str], cx: &mut VisualTestContext, ) -> Entity> { - cx.simulate_keystrokes("cmd-shift-p"); + cx.dispatch_action(Toggle); cx.run_until_parked(); let palette = workspace.update(cx, |workspace, cx| { diff --git a/crates/command_palette/src/command_palette_settings.rs b/crates/command_palette/src/command_palette_settings.rs new file mode 100644 index 0000000000..5763fcabfe --- /dev/null +++ b/crates/command_palette/src/command_palette_settings.rs @@ -0,0 +1,18 @@ +use settings::Settings; + +#[derive(Clone, Debug, PartialEq)] +pub struct CommandPaletteSettings { + pub use_command_history: bool, +} + +impl Settings for CommandPaletteSettings { + fn from_settings(content: &settings::SettingsContent) -> Self { + Self { + use_command_history: content + .command_palette + .as_ref() + .and_then(|settings| settings.use_command_history) + .expect("missing default for command_palette.use_command_history"), + } + } +} diff --git a/crates/command_palette/src/persistence.rs b/crates/command_palette/src/persistence.rs index 759d1204ee..3fb8c83c83 100644 --- a/crates/command_palette/src/persistence.rs +++ b/crates/command_palette/src/persistence.rs @@ -86,6 +86,12 @@ impl CommandPaletteDB { .await } + query! { + pub async fn delete_command_history(command_name: String) -> Result<()> { + DELETE FROM command_invocations WHERE command_name = (?) + } + } + #[cfg(test)] query! { pub(crate) fn get_last_invoked(command: &str) -> Result> { @@ -233,6 +239,33 @@ mod tests { assert_eq!(commands.as_slice()[1].invocations, 1); } + #[gpui::test] + async fn test_deletes_all_history_for_one_command() { + let db = CommandPaletteDB::open_test_db("test_deletes_all_history_for_one_command").await; + db.write_command_invocation("editor: backspace", "back") + .await + .unwrap(); + db.write_command_invocation("editor: backspace", "backspace") + .await + .unwrap(); + db.write_command_invocation("go to line: toggle", "line") + .await + .unwrap(); + let remaining_usage = db.get_command_usage("go to line: toggle").unwrap().unwrap(); + + for _ in 0..2 { + db.delete_command_history("editor: backspace".to_string()) + .await + .unwrap(); + + assert_eq!( + db.list_commands_used().unwrap(), + vec![remaining_usage.clone()] + ); + assert_eq!(db.list_recent_queries().unwrap(), vec!["line"]); + } + } + #[gpui::test] async fn test_handles_max_invocation_entries() { let db = CommandPaletteDB::open_test_db("test_handles_max_invocation_entries").await; diff --git a/crates/picker/src/picker.rs b/crates/picker/src/picker.rs index 3a93c7cd75..b05516775c 100644 --- a/crates/picker/src/picker.rs +++ b/crates/picker/src/picker.rs @@ -1261,6 +1261,9 @@ impl Picker { delegate_pending_update_matches.await; this.update_in(cx, |this, window, cx| { this.matches_updated(scroll_behavior, window, cx); + if let Some(secondary) = this.confirm_on_update.take() { + this.do_confirm(secondary, window, cx); + } }) }), }); @@ -1306,9 +1309,6 @@ impl Picker { { preview.update(update, window, cx); } - if let Some(secondary) = self.confirm_on_update.take() { - self.do_confirm(secondary, window, cx); - } cx.notify(); } @@ -1648,6 +1648,8 @@ mod tests { items: Vec, selected_index: usize, confirmed_index: Rc>>, + confirmations: Vec<(usize, bool)>, + match_update: Option<(Duration, usize)>, supports_multi_select: bool, selected_items: Vec, multi_confirmed: Rc>>>, @@ -1659,6 +1661,8 @@ mod tests { items, selected_index: 0, confirmed_index: Rc::new(Cell::new(None)), + confirmations: Vec::new(), + match_update: None, supports_multi_select: false, selected_items: Vec::new(), multi_confirmed: Rc::new(Cell::new(None)), @@ -1712,18 +1716,30 @@ mod tests { &mut self, _query: String, _window: &mut Window, - _cx: &mut Context>, + cx: &mut Context>, ) -> Task<()> { - Task::ready(()) + let Some((delay, selected_index)) = self.match_update.take() else { + return Task::ready(()); + }; + let timer = cx.background_executor().timer(delay); + cx.spawn(async move |picker, cx| { + timer.await; + picker + .update(cx, |picker, _| { + picker.delegate.selected_index = selected_index; + }) + .expect("picker should exist until matches finish"); + }) } fn confirm( &mut self, - _secondary: bool, + secondary: bool, _window: &mut Window, _cx: &mut Context>, ) { self.confirmed_index.set(Some(self.selected_index)); + self.confirmations.push((self.selected_index, secondary)); } fn supports_multi_select(&self) -> bool { @@ -1792,6 +1808,55 @@ mod tests { }); } + #[gpui::test] + async fn test_refresh_waits_for_latest_matches_before_confirming(cx: &mut TestAppContext) { + init_test(cx); + + for secondary in [false, true] { + let (picker, cx) = cx.add_window_view(|window, cx| { + Picker::uniform_list(TestDelegate::new(vec![true, true, true]), window, cx) + }); + cx.run_until_parked(); + + picker.update_in(cx, |picker, window, cx| { + picker.delegate.match_update = Some((Duration::from_millis(100), 1)); + picker.set_query("new query", window, cx); + }); + cx.run_until_parked(); + + picker.update_in(cx, |picker, window, cx| { + if secondary { + picker.secondary_confirm(&menu::SecondaryConfirm, window, cx); + } else { + picker.confirm(&menu::Confirm, window, cx); + } + assert_eq!(picker.confirm_on_update, Some(secondary)); + assert_eq!(picker.delegate.confirmations, Vec::new()); + + picker.delegate.match_update = Some((Duration::from_millis(50), 2)); + picker.refresh(window, cx); + assert_eq!(picker.delegate.confirmations, Vec::new()); + assert_eq!(picker.confirm_on_update, Some(secondary)); + }); + cx.run_until_parked(); + + cx.executor().advance_clock(Duration::from_millis(50)); + cx.run_until_parked(); + picker.read_with(cx, |picker, _| { + assert_eq!(picker.delegate.confirmations, vec![(2, secondary)]); + assert_eq!(picker.confirm_on_update, None); + assert!(picker.pending_update_matches.is_none()); + }); + + cx.executor().advance_clock(Duration::from_millis(100)); + cx.run_until_parked(); + picker.read_with(cx, |picker, _| { + assert_eq!(picker.delegate.selected_index(), 2); + assert_eq!(picker.delegate.confirmations, vec![(2, secondary)]); + }); + } + } + #[gpui::test] async fn test_clicking_non_selectable_item_does_not_confirm(cx: &mut TestAppContext) { init_test(cx); diff --git a/crates/settings/src/vscode_import.rs b/crates/settings/src/vscode_import.rs index fbf9df9c3a..997625f915 100644 --- a/crates/settings/src/vscode_import.rs +++ b/crates/settings/src/vscode_import.rs @@ -188,6 +188,11 @@ impl VsCodeSettings { base_keymap: Some(BaseKeymapContent::VSCode), calls: None, collaboration_panel: None, + command_palette: self + .read_u64("workbench.commandPalette.history") + .map(|history| CommandPaletteSettingsContent { + use_command_history: Some(history > 0), + }), credentials_url: None, debugger: None, diagnostics: None, @@ -1242,6 +1247,26 @@ mod tests { assert_eq!(imported_reduce_motion("{}"), None); } + #[test] + fn test_import_command_palette_history() { + for (content, expected) in [ + (r#"{ "workbench.commandPalette.history": 0 }"#, Some(false)), + (r#"{ "workbench.commandPalette.history": 1 }"#, Some(true)), + (r#"{ "workbench.commandPalette.history": 50 }"#, Some(true)), + ("{}", None), + ] { + let settings = VsCodeSettings::from_str(content, VsCodeSettingsSource::VsCode) + .unwrap() + .settings_content(); + assert_eq!( + settings + .command_palette + .and_then(|settings| settings.use_command_history), + expected, + ); + } + } + #[test] fn test_import_reveal_if_open() { let settings = VsCodeSettings::from_str( diff --git a/crates/settings_content/src/settings_content.rs b/crates/settings_content/src/settings_content.rs index 2f3efc04ca..8db6f134d7 100644 --- a/crates/settings_content/src/settings_content.rs +++ b/crates/settings_content/src/settings_content.rs @@ -190,6 +190,9 @@ pub struct SettingsContent { #[serde(flatten)] pub remote: RemoteSettingsContent, + /// Settings related to the command palette. + pub command_palette: Option, + /// Settings related to the file finder. pub file_finder: Option, @@ -399,7 +402,7 @@ impl SettingsContent { fallible_options::flattened_deserialize!(SettingsContent { sections: { project, theme, extension, workspace, editor, remote }, options: { - call_hierarchy, file_finder, git_panel, tabs, tab_bar, status_bar, preview_tabs, agent, + call_hierarchy, command_palette, file_finder, git_panel, tabs, tab_bar, status_bar, preview_tabs, agent, agent_servers, audio, auto_update, base_keymap, collaboration_panel, debugger, diagnostics, git, global_lsp_settings, image_viewer, markdown_preview, repl, helix_mode, hide_mouse, @@ -924,6 +927,15 @@ pub struct PanelSettingsContent { pub default_width: Option, } +#[with_fallible_options] +#[derive(Clone, Default, Serialize, Deserialize, JsonSchema, MergeFrom, Debug, PartialEq)] +pub struct CommandPaletteSettingsContent { + /// Whether to use command history ranking for sorting in the command palette. + /// + /// Default: true + pub use_command_history: Option, +} + #[with_fallible_options] #[derive(Clone, Default, Serialize, Deserialize, JsonSchema, MergeFrom, Debug, PartialEq)] pub struct FileFinderSettingsContent { diff --git a/crates/settings_ui/src/page_data.rs b/crates/settings_ui/src/page_data.rs index 1f76abe448..19e802a2a2 100644 --- a/crates/settings_ui/src/page_data.rs +++ b/crates/settings_ui/src/page_data.rs @@ -3832,6 +3832,35 @@ fn search_and_files_page() -> SettingsPage { ] } + fn command_palette_section() -> [SettingsPageItem; 2] { + [ + SettingsPageItem::SectionHeader("Command Palette"), + SettingsPageItem::SettingItem(SettingItem { + title: "Use Command History", + description: "Whether to use command history ranking for sorting in the command palette.", + field: Box::new(SettingField { + organization_override: None, + json_path: Some("command_palette.use_command_history"), + pick: |settings_content| { + settings_content + .command_palette + .as_ref()? + .use_command_history + .as_ref() + }, + write: |settings_content, value, _| { + settings_content + .command_palette + .get_or_insert_default() + .use_command_history = value; + }, + }), + metadata: None, + files: USER, + }), + ] + } + fn file_finder_section() -> [SettingsPageItem; 4] { [ SettingsPageItem::SectionHeader("File Finder"), @@ -4022,7 +4051,12 @@ fn search_and_files_page() -> SettingsPage { SettingsPage { title: "Search & Files", - items: concat_sections![search_section(), file_finder_section(), file_scan_section()], + items: concat_sections![ + search_section(), + command_palette_section(), + file_finder_section(), + file_scan_section(), + ], } } diff --git a/docs/src/reference/all-settings.md b/docs/src/reference/all-settings.md index 9ed6f5e6c0..40c909c526 100644 --- a/docs/src/reference/all-settings.md +++ b/docs/src/reference/all-settings.md @@ -552,6 +552,24 @@ When enabled, this setting will automatically close tabs for files that have bee } ``` +## Command Palette + +### Use Command History + +- Description: Whether to use command history ranking for sorting in the command palette. +- Setting: `command_palette.use_command_history` +- Default: `true` + +Disabling this setting does not erase history. + +```json [settings] +{ + "command_palette": { + "use_command_history": false + } +} +``` + ## Confirm Quit - Description: Whether or not to prompt the user to confirm before closing the application.