From fa4a7e7e38522d5ec3a0390eab88905f4067006c Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Sun, 23 Aug 2026 00:29:10 +0800 Subject: [PATCH] fix(keymap): say when a keybinding in config.json did nothing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two places throw away an entry in `config.json`'s `keybindings` map: `set_binding` when no action answers to the name, and `action_bindings` when the chord will not parse. Both say so with `log::warn!`, and per `docs/reference/privacy.mdx` there is no log at all unless `TTY7_LOG` or `RUST_LOG` is set. So on a default install a hand-edited typo costs the user their shortcut in silence. The file still parses, which means `tty7 doctor` reports config ok while two of the three bindings in it do nothing. The only evidence left is a key that never fires, and nothing connects that to the line that needs fixing — measured on a config carrying one typo'd action name, one unparseable chord, and one good binding. The window says it now, the same way it already says a config.json did not parse at all, and for the same reason: the symptom on its own reads as "tty7 ignored my settings". It is answered in the window rather than in `doctor` because `doctor` cannot ask. The action table and the keystroke parser both live up here; the CLI shares only the crate underneath, and moving either down to answer one diagnostic would be the tail wagging the dog. Two things are deliberately not faults. An empty chord is how a binding is *unbound* — `action_bindings` skips it on purpose. And a quarantined config is running on defaults, so its `keybindings` were never read; complaining about them would name a map nothing consulted. The collector is tested against all four cases, and checked against the log the two drop-sites emit: it names exactly the pair they drop and not the binding that works. --- src/main.rs | 44 +++++++++++++++++++++ src/ui/i18n/en.rs | 3 ++ src/ui/i18n/ja.rs | 3 ++ src/ui/i18n/mod.rs | 1 + src/ui/i18n/zh.rs | 3 ++ src/ui/keymap.rs | 99 ++++++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 153 insertions(+) diff --git a/src/main.rs b/src/main.rs index 1d813082..ab783fca 100644 --- a/src/main.rs +++ b/src/main.rs @@ -217,6 +217,44 @@ fn notify_config_load_failed( }); } +/// Says out loud that some of the `keybindings` in config.json did nothing. +/// +/// The two places that drop one already say so with `log::warn!`, and there is +/// no log unless `TTY7_LOG` is set — so on a default install a typo'd action +/// name or an unparseable chord costs the user their shortcut in silence. +/// `config.json` still parses, so `tty7 doctor` reports it `ok`, and the only +/// evidence is a key that does nothing. +/// +/// Same shape as the config notice above, for the same reason: the symptom on +/// its own reads as "tty7 ignored my settings", and nothing points at the line +/// that needs fixing. +fn notify_ignored_keybindings(cx: &mut App) { + use gpui_component::WindowExt as _; + + let faults = crate::ui::keymap::ignored_keybindings(cx); + if faults.is_empty() { + return; + } + let names = faults + .iter() + .map(|(action, _)| action.as_str()) + .collect::>() + .join(", "); + let text = crate::ui::i18n::t_fmt( + crate::ui::i18n::L10nKey::KeybindingsIgnored, + &[("count", &faults.len().to_string()), ("names", &names)], + ); + let Some(workspace) = crate::ui::windows::WindowRegistry::most_recent(cx) else { + return; + }; + let Some(handle) = crate::ui::windows::WindowRegistry::window_for(cx, workspace) else { + return; + }; + let _ = handle.update(cx, |_, window, cx| { + window.push_notification(text, cx); + }); +} + fn strip_os_arg_prefix(arg: &std::ffi::OsStr, prefix: &str) -> Option { let suffix = arg.as_encoded_bytes().strip_prefix(prefix.as_bytes())?; // SAFETY: `prefix` is ASCII and is removed only from the beginning of an @@ -624,6 +662,12 @@ fn main() { if config_outcome.failed() { notify_config_load_failed(cx, config_outcome, true); } + // Only when the file itself loaded: a quarantined config is running + // on defaults, so its `keybindings` were never read and complaining + // about them would name a map nothing consulted. + else { + notify_ignored_keybindings(cx); + } }); } diff --git a/src/ui/i18n/en.rs b/src/ui/i18n/en.rs index 513ccde8..a067c566 100644 --- a/src/ui/i18n/en.rs +++ b/src/ui/i18n/en.rs @@ -53,6 +53,9 @@ pub fn translate_en(key: L10nKey) -> &'static str { "Type \"yes\" to override and trust the new key, or Esc to abort." } L10nKey::Override => "Override", + L10nKey::KeybindingsIgnored => { + "{count} of the keybindings in config.json did nothing and were skipped: {names}. Settings → Keybindings lists every action name and the chords they take." + } L10nKey::RememberKeychain => "Remember (keychain)", L10nKey::Cancel => "Cancel", L10nKey::Close => "Close", diff --git a/src/ui/i18n/ja.rs b/src/ui/i18n/ja.rs index d8f19204..a55f6e24 100644 --- a/src/ui/i18n/ja.rs +++ b/src/ui/i18n/ja.rs @@ -55,6 +55,9 @@ pub fn translate_ja(key: L10nKey) -> Option<&'static str> { "「yes」を入力すると新しいキーを上書きして信頼します。中止するには Esc を押してください" } L10nKey::Override => "上書き", + L10nKey::KeybindingsIgnored => { + "config.json のキーバインド {count} 件が無効のためスキップされました: {names}。設定 → キーバインド にすべてのアクション名とキーの書き方があります。" + } L10nKey::RememberKeychain => "キーチェーンに保存", L10nKey::Cancel => "キャンセル", L10nKey::Close => "閉じる", diff --git a/src/ui/i18n/mod.rs b/src/ui/i18n/mod.rs index 4043b548..0c064214 100644 --- a/src/ui/i18n/mod.rs +++ b/src/ui/i18n/mod.rs @@ -127,6 +127,7 @@ l10n_keys! { Abort, HostKeyOverrideMessage, Override, + KeybindingsIgnored, RememberKeychain, Cancel, Close, diff --git a/src/ui/i18n/zh.rs b/src/ui/i18n/zh.rs index a6ae4832..4b4a5724 100644 --- a/src/ui/i18n/zh.rs +++ b/src/ui/i18n/zh.rs @@ -49,6 +49,9 @@ pub fn translate_zh(key: L10nKey) -> Option<&'static str> { L10nKey::Abort => "中止", L10nKey::HostKeyOverrideMessage => "输入 yes 覆盖并信任新密钥，或按 Esc 中止。", L10nKey::Override => "覆盖", + L10nKey::KeybindingsIgnored => { + "config.json 中有 {count} 项快捷键无效，已跳过：{names}。设置 → 快捷键 中列出了所有可用的动作名和按键写法。" + } L10nKey::RememberKeychain => "记住（钥匙串）", L10nKey::Cancel => "取消", L10nKey::Close => "关闭", diff --git a/src/ui/keymap.rs b/src/ui/keymap.rs index d3724353..b9442bd7 100644 --- a/src/ui/keymap.rs +++ b/src/ui/keymap.rs @@ -822,6 +822,47 @@ fn authored_entry(action: &str) -> Option<(CommandGroup, String)> { }) } +/// Why an entry in `config.json`'s `keybindings` did nothing. +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +pub(crate) enum KeybindingFault { + /// No action goes by that name — a typo, or a name from another app. + UnknownAction, + /// The chord does not parse, so nothing could ever press it. + InvalidKeystroke, +} + +/// The entries in `config.json`'s `keybindings` map that were thrown away. +/// +/// Both halves already say so — `set_binding` for a name no action answers to, +/// `action_bindings` for a chord that does not parse — and both say it with +/// `log::warn!`, into a log that per `docs/reference/privacy.mdx` is not +/// written at all unless `TTY7_LOG` is set. So on a default install a +/// hand-edited typo costs the user their binding in silence: `config.json` +/// parses, `tty7 doctor` reports the config as `ok`, and the shortcut simply +/// never fires. +/// +/// It is answered here rather than in `doctor` because `doctor` cannot ask. +/// The action table and the keystroke parser both live in the window; the CLI +/// shares only the crate underneath, and moving either down there to answer +/// one diagnostic would be the tail wagging the dog. +/// +/// An empty chord is not a fault: that is how a binding is *unbound*, and +/// `action_bindings` skips it deliberately. +pub(crate) fn ignored_keybindings(cx: &App) -> Vec<(String, KeybindingFault)> { + let cfg = cx.global::(); + let known: Vec<&str> = default_bindings().into_iter().map(|(a, _)| a).collect(); + let mut out = Vec::new(); + for (action, key) in &cfg.keybindings { + if !known.contains(&action.as_str()) { + out.push((action.clone(), KeybindingFault::UnknownAction)); + } else if !key.is_empty() && !keystroke_is_valid(key) { + out.push((action.clone(), KeybindingFault::InvalidKeystroke)); + } + } + out.sort_by(|a, b| a.0.cmp(&b.0)); + out +} + pub(crate) fn effective_bindings(cx: &App) -> Vec<(String, String)> { let cfg = cx.global::(); let mut effective: Vec<(String, String)> = default_bindings() @@ -1896,6 +1937,64 @@ mod gpui_tests { use crate::core::config::Config; use gpui::TestAppContext; + /// What a hand-edited `keybindings` map loses, and why anyone is told. + /// + /// `set_binding` drops a name no action answers to and `action_bindings` + /// drops a chord that will not parse — both with `log::warn!`, into a log + /// that is not written unless `TTY7_LOG` is set. `config.json` still + /// parses either way, so `tty7 doctor` calls it `ok` and the only evidence + /// left is a shortcut that never fires. + /// + /// An empty chord is not a fault: that is how a binding is unbound, and + /// `action_bindings` skips it on purpose. + #[gpui::test] + fn a_keybinding_that_does_nothing_is_collected_for_the_notice(cx: &mut TestAppContext) { + cx.update(|cx| { + gpui_component::init(cx); + cx.set_global(Config::default()); + init(cx); + { + let cfg = cx.global_mut::(); + // A real action with a real chord: nothing to report. + cfg.keybindings + .insert("SplitRight".into(), "secondary-shift-9".into()); + // A name no action answers to — the ordinary typo. + cfg.keybindings + .insert("NewTabb".into(), "secondary-t".into()); + // A real action whose chord cannot be pressed. + cfg.keybindings + .insert("NewWorkspace".into(), "not-a-chord".into()); + // Unbinding, which is a choice rather than a mistake. + cfg.keybindings.insert("RenameTab".into(), String::new()); + } + + let faults = ignored_keybindings(cx); + assert_eq!( + faults, + vec![ + ("NewTabb".to_string(), KeybindingFault::UnknownAction), + ( + "NewWorkspace".to_string(), + KeybindingFault::InvalidKeystroke + ), + ], + "the working binding and the deliberate unbind are not faults" + ); + }); + } + + /// A config nobody edited reports nothing, so the notice cannot cry wolf + /// on every launch. + #[gpui::test] + fn a_default_config_has_no_ignored_keybindings(cx: &mut TestAppContext) { + cx.update(|cx| { + gpui_component::init(cx); + cx.set_global(Config::default()); + init(cx); + assert!(ignored_keybindings(cx).is_empty()); + }); + } + #[gpui::test] fn init_then_rebind_installs_the_merged_table(cx: &mut TestAppContext) { cx.update(|cx| {