From f66dd43a847fb52297045fd14487dd5f58b05fa4 Mon Sep 17 00:00:00 2001 From: Piotr Osiewicz <24362066+osiewicz@users.noreply.github.com> Date: Mon, 11 Sep 2023 17:50:09 +0200 Subject: [PATCH] Z 2620 - Sort command palette's entries by name and use count (#2954) Release Notes: - Improved command palette's command ordering; commands are sorted lexicographically and by command's use count in the current session. --- crates/command_palette/src/command_palette.rs | 45 ++++++++++++++++--- 1 file changed, 40 insertions(+), 5 deletions(-) diff --git a/crates/command_palette/src/command_palette.rs b/crates/command_palette/src/command_palette.rs index 101d4dc545..4f9bb231ce 100644 --- a/crates/command_palette/src/command_palette.rs +++ b/crates/command_palette/src/command_palette.rs @@ -1,11 +1,11 @@ -use collections::CommandPaletteFilter; +use collections::{CommandPaletteFilter, HashMap}; use fuzzy::{StringMatch, StringMatchCandidate}; use gpui::{ actions, anyhow::anyhow, elements::*, keymap_matcher::Keystroke, Action, AnyWindowHandle, AppContext, Element, MouseState, ViewContext, }; use picker::{Picker, PickerDelegate, PickerEvent}; -use std::cmp; +use std::cmp::{self, Reverse}; use util::ResultExt; use workspace::Workspace; @@ -33,13 +33,18 @@ pub enum Event { action: Box, }, } - struct Command { name: String, action: Box, keystrokes: Vec, } +/// 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 an user already knows a keystroke for a command, they are unlikely to use a command palette to look for it. +#[derive(Default)] +struct HitCounts(HashMap); + fn toggle_command_palette(workspace: &mut Workspace, _: &Toggle, cx: &mut ViewContext) { let focused_view_id = cx.focused_view_id().unwrap_or_else(|| cx.view_id()); workspace.toggle_modal(cx, |_, cx| { @@ -83,7 +88,7 @@ impl PickerDelegate for CommandPaletteDelegate { let view_id = self.focused_view_id; let window = cx.window(); cx.spawn(move |picker, mut cx| async move { - let actions = window + let mut actions = window .available_actions(view_id, &cx) .into_iter() .flatten() @@ -112,6 +117,16 @@ impl PickerDelegate for CommandPaletteDelegate { } }) .collect::>(); + let actions = cx.read(move |cx| { + let hit_counts = cx.optional_global::(); + actions.sort_by_key(|action| { + ( + Reverse(hit_counts.and_then(|map| map.0.get(&action.name)).cloned()), + action.name.clone(), + ) + }); + actions + }); let candidates = actions .iter() .enumerate() @@ -166,7 +181,12 @@ impl PickerDelegate for CommandPaletteDelegate { let window = cx.window(); let focused_view_id = self.focused_view_id; let action_ix = self.matches[self.selected_ix].candidate_id; - let action = self.actions.remove(action_ix).action; + let command = self.actions.remove(action_ix); + cx.update_default_global(|hit_counts: &mut HitCounts, _| { + *hit_counts.0.entry(command.name).or_default() += 1; + }); + let action = command.action; + cx.app_context() .spawn(move |mut cx| async move { window @@ -319,6 +339,21 @@ mod tests { workspace.modal::().unwrap() }); + palette + .update(cx, |palette, cx| { + // Fill up palette's command list by running an empty query; + // we only need it to subsequently assert that the palette is initially + // sorted by command's name. + palette.delegate_mut().update_matches("".to_string(), cx) + }) + .await; + + palette.update(cx, |palette, _| { + let is_sorted = + |actions: &[Command]| actions.windows(2).all(|pair| pair[0].name <= pair[1].name); + assert!(is_sorted(&palette.delegate().actions)); + }); + palette .update(cx, |palette, cx| { palette