From 16f625bd07eacd071ce21f669b6baf4847e20ab6 Mon Sep 17 00:00:00 2001 From: KyleBarton Date: Tue, 1 Apr 2025 14:46:35 -0700 Subject: [PATCH] Add persistence to command palette history (#26948) Closes #20391 ### Summary This adds a persistence layer to the command palette so that usages can persist after Zed is closed and re-opened. The current "usage" algorithm is unchanged, e.g.: - Sorts by number of usages descending (no recency preference) - Once a user's query is active, removes these suggestions in favor of fuzzy matching There are some additional considerations in order to keep the DB from growing uncontrollably (and to make long-term use ergonomic): - The "invocations" count handles max values (though at u16, it seems unlikely a user will deal with this) - If a command is un-invoked for more than a month, it stops being considered a recent usage, and its next update will update its usages back to 1 ### Future Considerations - Could make the "command expiry" configurable in settings, so the user can decide how long to hold onto recent usages - Could make a more sophisticated algorithm which balances recency and total invocations - e.g. if I've used COMMAND_A 100 times in the last month, but COMMAND_B 10 times today, should COMMAND_B actually be preferred? - Could do preferential fuzzy-matching against these matches once the user starts a query. Release Notes: - Added persistent history of command palette usages. --------- Co-authored-by: Peter Finn Co-authored-by: Conrad Irwin --- Cargo.lock | 3 + crates/command_palette/Cargo.toml | 4 + crates/command_palette/src/command_palette.rs | 51 +++-- crates/command_palette/src/persistence.rs | 210 ++++++++++++++++++ 4 files changed, 251 insertions(+), 17 deletions(-) create mode 100644 crates/command_palette/src/persistence.rs diff --git a/Cargo.lock b/Cargo.lock index b13fbbf33f..81ea8f3865 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3113,10 +3113,12 @@ dependencies = [ name = "command_palette" version = "0.1.0" dependencies = [ + "anyhow", "client", "collections", "command_palette_hooks", "ctor", + "db", "editor", "env_logger 0.11.7", "fuzzy", @@ -3132,6 +3134,7 @@ dependencies = [ "settings", "telemetry", "theme", + "time", "ui", "util", "workspace", diff --git a/crates/command_palette/Cargo.toml b/crates/command_palette/Cargo.toml index fd7fccebeb..3b09ef8f86 100644 --- a/crates/command_palette/Cargo.toml +++ b/crates/command_palette/Cargo.toml @@ -13,9 +13,11 @@ path = "src/command_palette.rs" doctest = false [dependencies] +anyhow.workspace = true client.workspace = true collections.workspace = true command_palette_hooks.workspace = true +db.workspace = true fuzzy.workspace = true gpui.workspace = true picker.workspace = true @@ -23,6 +25,7 @@ postage.workspace = true serde.workspace = true settings.workspace = true theme.workspace = true +time.workspace = true ui.workspace = true util.workspace = true telemetry.workspace = true @@ -31,6 +34,7 @@ zed_actions.workspace = true [dev-dependencies] ctor.workspace = true +db = { workspace = true, features = ["test-support"] } editor = { workspace = true, features = ["test-support"] } env_logger.workspace = true go_to_line.workspace = true diff --git a/crates/command_palette/src/command_palette.rs b/crates/command_palette/src/command_palette.rs index 7a2db51fb2..4b19ee8300 100644 --- a/crates/command_palette/src/command_palette.rs +++ b/crates/command_palette/src/command_palette.rs @@ -1,19 +1,23 @@ +mod persistence; + use std::{ cmp::{self, Reverse}, + collections::HashMap, sync::Arc, time::Duration, }; use client::parse_zed_link; -use collections::HashMap; use command_palette_hooks::{ CommandInterceptResult, CommandPaletteFilter, CommandPaletteInterceptor, }; + use fuzzy::{StringMatch, StringMatchCandidate}; use gpui::{ - Action, App, Context, DismissEvent, Entity, EventEmitter, FocusHandle, Focusable, Global, - ParentElement, Render, Styled, Task, UpdateGlobal, WeakEntity, Window, + Action, App, Context, DismissEvent, Entity, EventEmitter, FocusHandle, Focusable, + ParentElement, Render, Styled, Task, WeakEntity, Window, }; +use persistence::COMMAND_PALETTE_HISTORY; use picker::{Picker, PickerDelegate}; use postage::{sink::Sink, stream::Stream}; use settings::Settings; @@ -24,7 +28,6 @@ use zed_actions::{OpenZedUrl, command_palette::Toggle}; pub fn init(cx: &mut App) { client::init_settings(cx); - cx.set_global(HitCounts::default()); command_palette_hooks::init(cx); cx.observe_new(CommandPalette::register).detach(); } @@ -138,6 +141,7 @@ impl Render for CommandPalette { } pub struct CommandPaletteDelegate { + latest_query: String, command_palette: WeakEntity, all_commands: Vec, commands: Vec, @@ -164,14 +168,6 @@ impl Clone for Command { } } -/// 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. -#[derive(Default, Clone)] -struct HitCounts(HashMap); - -impl Global for HitCounts {} - impl CommandPaletteDelegate { fn new( command_palette: WeakEntity, @@ -185,6 +181,7 @@ impl CommandPaletteDelegate { commands, selected_ix: 0, previous_focus_handle, + latest_query: String::new(), updating_matches: None, } } @@ -197,6 +194,7 @@ impl CommandPaletteDelegate { cx: &mut Context>, ) { self.updating_matches.take(); + self.latest_query = query.clone(); let mut intercept_results = CommandPaletteInterceptor::try_global(cx) .map(|interceptor| interceptor.intercept(&query, cx)) @@ -244,6 +242,20 @@ impl CommandPaletteDelegate { self.selected_ix = cmp::min(self.selected_ix, self.matches.len() - 1); } } + /// + /// 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 hit_counts(&self) -> HashMap { + if let Ok(commands) = COMMAND_PALETTE_HISTORY.list_commands_used() { + commands + .into_iter() + .map(|command| (command.command_name, command.invocations)) + .collect() + } else { + HashMap::new() + } + } } impl PickerDelegate for CommandPaletteDelegate { @@ -283,13 +295,13 @@ impl PickerDelegate for CommandPaletteDelegate { let (mut tx, mut rx) = postage::dispatch::channel(1); let task = cx.background_spawn({ let mut commands = self.all_commands.clone(); - let hit_counts = cx.global::().clone(); + let hit_counts = self.hit_counts(); let executor = cx.background_executor().clone(); let query = normalize_query(query.as_str()); async move { commands.sort_by_key(|action| { ( - Reverse(hit_counts.0.get(&action.name).cloned()), + Reverse(hit_counts.get(&action.name).cloned()), action.name.clone(), ) }); @@ -388,9 +400,14 @@ impl PickerDelegate for CommandPaletteDelegate { ); self.matches.clear(); self.commands.clear(); - HitCounts::update_global(cx, |hit_counts, _cx| { - *hit_counts.0.entry(command.name).or_default() += 1; - }); + let command_name = command.name.clone(); + let latest_query = self.latest_query.clone(); + cx.background_spawn(async move { + COMMAND_PALETTE_HISTORY + .write_command_invocation(command_name, latest_query) + .await + }) + .detach_and_log_err(cx); let action = command.action; window.focus(&self.previous_focus_handle); self.dismissed(window, cx); diff --git a/crates/command_palette/src/persistence.rs b/crates/command_palette/src/persistence.rs new file mode 100644 index 0000000000..f750f0f871 --- /dev/null +++ b/crates/command_palette/src/persistence.rs @@ -0,0 +1,210 @@ +use anyhow::Result; +use db::{ + define_connection, query, + sqlez::{bindable::Column, statement::Statement}, + sqlez_macros::sql, +}; +use serde::{Deserialize, Serialize}; +use time::OffsetDateTime; + +#[derive(Debug, Clone, Deserialize, Serialize, PartialEq)] +pub(crate) struct SerializedCommandInvocation { + pub(crate) command_name: String, + pub(crate) user_query: String, + pub(crate) last_invoked: OffsetDateTime, +} + +#[derive(Debug, Clone, Deserialize, Serialize, PartialEq)] +pub(crate) struct SerializedCommandUsage { + pub(crate) command_name: String, + pub(crate) invocations: u16, + pub(crate) last_invoked: OffsetDateTime, +} + +impl Column for SerializedCommandUsage { + fn column(statement: &mut Statement, start_index: i32) -> Result<(Self, i32)> { + let (command_name, next_index): (String, i32) = Column::column(statement, start_index)?; + let (invocations, next_index): (u16, i32) = Column::column(statement, next_index)?; + let (last_invoked_raw, next_index): (i64, i32) = Column::column(statement, next_index)?; + + let usage = Self { + command_name, + invocations, + last_invoked: OffsetDateTime::from_unix_timestamp(last_invoked_raw)?, + }; + Ok((usage, next_index)) + } +} + +impl Column for SerializedCommandInvocation { + fn column(statement: &mut Statement, start_index: i32) -> Result<(Self, i32)> { + let (command_name, next_index): (String, i32) = Column::column(statement, start_index)?; + let (user_query, next_index): (String, i32) = Column::column(statement, next_index)?; + let (last_invoked_raw, next_index): (i64, i32) = Column::column(statement, next_index)?; + let command_invocation = Self { + command_name, + user_query, + last_invoked: OffsetDateTime::from_unix_timestamp(last_invoked_raw)?, + }; + Ok((command_invocation, next_index)) + } +} + +define_connection!(pub static ref COMMAND_PALETTE_HISTORY: CommandPaletteDB<()> = + &[sql!( + CREATE TABLE IF NOT EXISTS command_invocations( + id INTEGER PRIMARY KEY AUTOINCREMENT, + command_name TEXT NOT NULL, + user_query TEXT NOT NULL, + last_invoked INTEGER DEFAULT (unixepoch()) NOT NULL + ) STRICT; + )]; +); + +impl CommandPaletteDB { + pub async fn write_command_invocation( + &self, + command_name: impl Into, + user_query: impl Into, + ) -> Result<()> { + self.write_command_invocation_internal(command_name.into(), user_query.into()) + .await + } + + query! { + pub fn get_last_invoked(command: &str) -> Result> { + SELECT + command_name, + user_query, + last_invoked FROM command_invocations + WHERE command_name=(?) + ORDER BY last_invoked DESC + LIMIT 1 + } + } + + query! { + pub fn get_command_usage(command: &str) -> Result> { + SELECT command_name, COUNT(1), MAX(last_invoked) + FROM command_invocations + WHERE command_name=(?) + GROUP BY command_name + } + } + + query! { + async fn write_command_invocation_internal(command_name: String, user_query: String) -> Result<()> { + INSERT INTO command_invocations (command_name, user_query) VALUES ((?), (?)); + DELETE FROM command_invocations WHERE id IN (SELECT MIN(id) FROM command_invocations HAVING COUNT(1) > 1000); + } + } + + query! { + pub fn list_commands_used() -> Result> { + SELECT command_name, COUNT(1), MAX(last_invoked) + FROM command_invocations + GROUP BY command_name + ORDER BY COUNT(1) DESC + } + } +} + +#[cfg(test)] +mod tests { + + use crate::persistence::{CommandPaletteDB, SerializedCommandUsage}; + + #[gpui::test] + async fn test_saves_and_retrieves_command_invocation() { + let db = + CommandPaletteDB(db::open_test_db("test_saves_and_retrieves_command_invocation").await); + + let retrieved_cmd = db.get_last_invoked("editor: backspace").unwrap(); + + assert!(retrieved_cmd.is_none()); + + db.write_command_invocation("editor: backspace", "") + .await + .unwrap(); + + let retrieved_cmd = db.get_last_invoked("editor: backspace").unwrap(); + + assert!(retrieved_cmd.is_some()); + let retrieved_cmd = retrieved_cmd.expect("is some"); + assert_eq!(retrieved_cmd.command_name, "editor: backspace".to_string()); + assert_eq!(retrieved_cmd.user_query, "".to_string()); + } + + #[gpui::test] + async fn test_gets_usage_history() { + let db = CommandPaletteDB(db::open_test_db("test_gets_usage_history").await); + db.write_command_invocation("go to line: toggle", "200") + .await + .unwrap(); + db.write_command_invocation("go to line: toggle", "201") + .await + .unwrap(); + + let retrieved_cmd = db.get_last_invoked("go to line: toggle").unwrap(); + + assert!(retrieved_cmd.is_some()); + let retrieved_cmd = retrieved_cmd.expect("is some"); + + let command_usage = db.get_command_usage("go to line: toggle").unwrap(); + + assert!(command_usage.is_some()); + let command_usage: SerializedCommandUsage = command_usage.expect("is some"); + + assert_eq!(command_usage.command_name, "go to line: toggle"); + assert_eq!(command_usage.invocations, 2); + assert_eq!(command_usage.last_invoked, retrieved_cmd.last_invoked); + } + + #[gpui::test] + async fn test_lists_ordered_by_usage() { + let db = CommandPaletteDB(db::open_test_db("test_lists_ordered_by_usage").await); + + let empty_commands = db.list_commands_used(); + match &empty_commands { + Ok(_) => (), + Err(e) => println!("Error: {:?}", e), + } + assert!(empty_commands.is_ok()); + assert_eq!(empty_commands.expect("is ok").len(), 0); + + db.write_command_invocation("go to line: toggle", "200") + .await + .unwrap(); + db.write_command_invocation("editor: backspace", "") + .await + .unwrap(); + db.write_command_invocation("editor: backspace", "") + .await + .unwrap(); + + let commands = db.list_commands_used(); + + assert!(commands.is_ok()); + let commands = commands.expect("is ok"); + assert_eq!(commands.len(), 2); + assert_eq!(commands.as_slice()[0].command_name, "editor: backspace"); + assert_eq!(commands.as_slice()[0].invocations, 2); + assert_eq!(commands.as_slice()[1].command_name, "go to line: toggle"); + assert_eq!(commands.as_slice()[1].invocations, 1); + } + + #[gpui::test] + async fn test_handles_max_invocation_entries() { + let db = CommandPaletteDB(db::open_test_db("test_handles_max_invocation_entries").await); + + for i in 1..=1001 { + db.write_command_invocation("some-command", &i.to_string()) + .await + .unwrap(); + } + let some_command = db.get_command_usage("some-command").unwrap(); + + assert!(some_command.is_some()); + assert_eq!(some_command.expect("is some").invocations, 1000); + } +}