mirror of
https://github.com/l0ng-ai/tty7.git
synced 2026-09-22 00:02:23 +00:00
fix(keymap): say when a keybinding in config.json did nothing
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.
This commit is contained in:
+44
@@ -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::<Vec<_>>()
|
||||
.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<std::ffi::OsString> {
|
||||
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);
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -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 => "閉じる",
|
||||
|
||||
@@ -127,6 +127,7 @@ l10n_keys! {
|
||||
Abort,
|
||||
HostKeyOverrideMessage,
|
||||
Override,
|
||||
KeybindingsIgnored,
|
||||
RememberKeychain,
|
||||
Cancel,
|
||||
Close,
|
||||
|
||||
@@ -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 => "关闭",
|
||||
|
||||
@@ -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::<Config>();
|
||||
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::<Config>();
|
||||
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::<Config>();
|
||||
// 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| {
|
||||
|
||||
Reference in New Issue
Block a user