From b5f8d484bad3814e253d82d395e9a1b665cf8d2f Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Sat, 22 Aug 2026 21:46:06 +0800 Subject: [PATCH] fix(palette): stop naming a keymap action the keymap has never had MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `key_spec` maps a palette command to the keymap action whose shortcut the row should show, and `effective_key` answers an action it does not know the same way it answers a deliberately unbound one: `None`. So a name that resolves to nothing does not fail — it quietly shows no shortcut, which is indistinguishable from having none. `RestartDaemon` named `"RestartDaemon"`, and the keymap has never bound it. Nothing was visibly wrong today, because the command has no shortcut either way; what was wrong is that the palette claimed a bindable action, so binding one later would still have shown nothing. It moves to the arm for commands with no action of their own. Restarting the server stays palette-only: giving it a real action means a gpui action and a handler, which is a feature rather than a fix. The guard reads the names out of `key_spec`'s own source, because the match *is* the list and a second copy here would drift the way the first one did. Both it and the settings-index guard were checked against an injected regression rather than only against a green tree — a phantom action and a removed index entry each fail them. --- src/ui/palette.rs | 65 ++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 64 insertions(+), 1 deletion(-) diff --git a/src/ui/palette.rs b/src/ui/palette.rs index 6fa37938..696a389b 100644 --- a/src/ui/palette.rs +++ b/src/ui/palette.rs @@ -279,7 +279,6 @@ impl CommandKind { OpenDiscord => "OpenDiscord", ReportIssue => "ReportIssue", Quit => "Quit", - RestartDaemon => "RestartDaemon", ToggleSftp => "ToggleSftp", ShowSshForwards => "ShowSshForwards", ToggleCodePanel => "ToggleCodePanel", @@ -300,6 +299,13 @@ impl CommandKind { ScmCreateBranch => "ScmCreateBranch", OpenBranchPicker => "ScmCheckoutBranch", ToggleDiffViewMode => "ToggleDiffViewMode", + // No keymap action of their own, so there is no shortcut to show. + // `RestartDaemon` is here rather than above because it named + // `"RestartDaemon"`, which the keymap has never bound: the lookup + // could only ever come back empty, and naming an action that does + // not exist reads like one that does. Restarting the server is + // palette-only for now; giving it a bindable action means a gpui + // action and a handler, which is a feature rather than a fix. CopyText | CutText | PasteText @@ -310,6 +316,7 @@ impl CommandKind { | OpenThemePicker | OpenSshConnectInput | OpenSshConnect(_) + | RestartDaemon | SetTheme(_) | ActivateTab(_) | ConnectSavedProfile(_) @@ -1557,6 +1564,62 @@ mod gpui_tests { use super::*; use gpui::TestAppContext; + /// Every keymap action `key_spec` names is one the keymap actually binds. + /// + /// `effective_key` answers an action it does not know with `None`, exactly + /// as it answers one that is deliberately unbound — so a misspelled or + /// renamed action does not fail, it just quietly stops showing the shortcut + /// it was written to show. `RestartDaemon` named an action the keymap has + /// never had, and nothing said so. + /// + /// The names are read out of `key_spec`'s own source: the match is the + /// list, and a second copy of it here would drift the way the first one did. + #[test] + fn key_spec_only_names_actions_the_keymap_binds() { + const SOURCE: &str = include_str!("palette.rs"); + + let body = { + let start = SOURCE + .find("fn key_spec(&self, cx: &App) -> Option {") + .expect("key_spec is still spelled this way"); + let rest = &SOURCE[start..]; + let end = rest.find("\n }\n").expect("key_spec has a body"); + &rest[..end] + }; + + let named: Vec<&str> = body + .match_indices("=> \"") + .map(|(at, _)| { + let from = &body[at + 4..]; + &from[..from.find('"').expect("a closing quote")] + }) + // The inline `secondary-*` specs are keystrokes, not action names. + .filter(|s| !s.contains('-')) + .collect(); + assert!( + named.len() > 50, + "the source scan found only {} actions, so it has stopped matching \ + how `key_spec` is written", + named.len() + ); + + let bound: std::collections::HashSet<&str> = crate::ui::keymap::default_bindings() + .into_iter() + .map(|(action, _)| action) + .collect(); + + let mut phantom: Vec<&str> = named.into_iter().filter(|a| !bound.contains(a)).collect(); + phantom.sort(); + phantom.dedup(); + + assert!( + phantom.is_empty(), + "`key_spec` names these actions, but the keymap binds none of them, \ + so the palette shows no shortcut for them however they are bound: \ + {phantom:?}" + ); + } + #[gpui::test] fn every_palette_command_has_a_stable_id(cx: &mut TestAppContext) { crate::core::config::pin_test_config_dir();