diff --git a/crates/tty7-core/src/core/config.rs b/crates/tty7-core/src/core/config.rs index 4058a7b4..06d1f6e4 100644 --- a/crates/tty7-core/src/core/config.rs +++ b/crates/tty7-core/src/core/config.rs @@ -167,6 +167,15 @@ pub struct Config { pub window_backdrop: WindowBackdrop, #[serde(default = "default_true")] pub dim_inactive_panes: bool, + /// Lenient one entry at a time, for the same reason the nested keys below + /// are: this is hand-edited, and it used to be all-or-nothing. A single + /// value serde could not read — `"ActivateTab1": null`, a number, an object + /// — failed the whole `Config`, which quarantines `config.json` and starts + /// the app on built-in defaults; every rebinding in the file then read as + /// its shipped default, and the next settings write persisted those + /// defaults over what the user wrote (#901). A line that cannot be read is + /// skipped with a warning, and the rest of the map still binds. + #[serde(default, deserialize_with = "de_keybindings")] pub keybindings: HashMap, #[serde(default = "default_preset")] pub keybinding_preset: String, @@ -1213,6 +1222,37 @@ fn default_sidebar_width() -> f32 { pub const MAX_SCROLLBACK: usize = 100_000; +/// `keybindings`, read one line at a time. +/// +/// [`de_lenient`] is all-or-nothing per field, which for a map means one bad +/// line throwing away every good one. Here each entry stands on its own: the +/// ones that name a shortcut or a list of shortcuts bind, the ones that do not +/// are logged and dropped. A `keybindings` that is not an object at all falls +/// back to an empty map rather than failing the file. +fn de_keybindings<'de, D>(deserializer: D) -> Result, D::Error> +where + D: serde::Deserializer<'de>, +{ + let value = serde_json::Value::deserialize(deserializer)?; + let serde_json::Value::Object(entries) = value else { + log::warn!("ignoring `keybindings` {value}: expected an object of action name to shortcut"); + return Ok(HashMap::new()); + }; + let mut bindings = HashMap::with_capacity(entries.len()); + for (action, raw) in entries { + match KeybindingOverride::deserialize(&raw) { + Ok(binding) => { + bindings.insert(action, binding); + } + Err(e) => log::warn!( + "ignoring keybinding for {action:?}: {raw} is not a shortcut, \ + a list of shortcuts, or \"\" to unbind ({e})" + ), + } + } + Ok(bindings) +} + pub(crate) fn de_lenient<'de, D, T>(deserializer: D) -> Result where D: serde::Deserializer<'de>, @@ -1968,6 +2008,34 @@ mod tests { assert_eq!(serde_json::to_value(&cfg.keybindings).unwrap(), written); } + #[test] + fn a_keybinding_line_that_cannot_be_read_does_not_take_the_config_with_it() { + // #901: one unreadable line used to fail the whole `Config`. The loader + // then quarantines config.json and starts on built-in defaults, so + // every rebinding in the file — the point of the file — came back as + // the shipped default, and the next settings write made that permanent. + let cfg: Config = serde_json::from_str( + r#"{"font_size": 20.0, + "keybindings": {"ActivateTab1": null, "ActivateTab2": 2, + "ActivateTab3": {"key": "alt-shift-3"}, + "ActivateTab4": [], "NextTab": "ctrl-alt-]"}}"#, + ) + .expect("a bad keybinding line must not fail the file"); + assert_eq!(cfg.font_size, 20.0, "the rest of the file still loads"); + assert_eq!( + serde_json::to_value(&cfg.keybindings).unwrap(), + serde_json::json!({"ActivateTab4": [], "NextTab": "ctrl-alt-]"}), + "the readable lines survive and the unreadable ones are dropped" + ); + + // And a `keybindings` that is not a map at all is no reason to hand + // the user back default fonts, themes and everything else. + let odd: Config = serde_json::from_str(r#"{"font_size": 20.0, "keybindings": []}"#) + .expect("a keybindings of the wrong shape must not fail the file"); + assert_eq!(odd.font_size, 20.0); + assert!(odd.keybindings.is_empty()); + } + fn pin_config_dir() { let dir = std::env::temp_dir().join(format!("tty7-covtest-{}", std::process::id())); std::fs::create_dir_all(&dir).ok(); diff --git a/docs/customization/keybindings.mdx b/docs/customization/keybindings.mdx index cc699d7c..26a4fa04 100644 --- a/docs/customization/keybindings.mdx +++ b/docs/customization/keybindings.mdx @@ -19,7 +19,13 @@ Click a shortcut and press the new keys. It saves after a brief pause. | Press keys | Set the binding | | Press more keys | Chain a sequence — ⌃ B then X | | Esc | Cancel | -| ⌫ | Remove the last key — or, pressed first, reset the shortcut to its default | +| ⌫ | Remove the last key — or, pressed first, leave the action with **no shortcut** | + +Leaving an action unbound is how you hand a key back to whatever is running in +the terminal: ⌥ 1…⌥ 9 jump between tabs by default, and +vim wants them for its own tabs. Clear the nine **Go to Tab** rows and the +digits go straight through. A cleared row shows `—` and grows a **Reset** +button, which puts the default back. **Restore all defaults** at the bottom undoes every rebinding at once. There is no undo for that one. @@ -74,8 +80,22 @@ space. | `cmd` · `ctrl` · `alt` · `shift` | Literal modifiers | | `ctrl-b n` | A two-key sequence | -An unknown action name or an invalid keystroke is skipped with a warning in the -log rather than breaking the rest of your bindings. +To keep a key for the program running in the terminal, unbind the action that +holds it. vim's tab keys, for instance: + +```json +{ + "keybindings": { + "ActivateTab1": [], "ActivateTab2": [], "ActivateTab3": [], + "ActivateTab4": [], "ActivateTab5": [], "ActivateTab6": [], + "ActivateTab7": [], "ActivateTab8": [], "ActivateTab9": [] + } +} +``` + +An unknown action name, an invalid keystroke, or a line that is neither a +shortcut nor a list of them is skipped with a warning in the log rather than +breaking the rest of your bindings. The full action list is on the [keyboard shortcuts](/reference/keyboard-shortcuts) page. diff --git a/src/ui/app.rs b/src/ui/app.rs index 717c7444..2022f520 100644 --- a/src/ui/app.rs +++ b/src/ui/app.rs @@ -7134,7 +7134,7 @@ impl Tty7App { cx.notify(); } else { self.stop_recording(cx); - self.reset_keybinding(action, cx); + self.unbind_keybinding(action, cx); } return; } @@ -7253,6 +7253,34 @@ impl Tty7App { cx.notify(); } + /// Takes every chord off an action, and keeps it off. + /// + /// ⌫ on a row that has recorded nothing used to *reset* it — drop the + /// override so the action gets its shipped chord back. On a row nobody has + /// overridden, which is every row the first time it is looked at, that is a + /// no-op: someone pressing Backspace over Alt+1 to be rid of it watched + /// Alt+1 sit exactly where it was and read it as the default restoring + /// itself (#901). Nothing anywhere in the app said "this action should have + /// no key", though `config.json` has spelled it `[]` since #868. + /// + /// So ⌫ writes that empty list, and the **Reset** button beside the row — + /// which appears the moment an action is overridden, this way included — is + /// the way back to the default. + pub(crate) fn unbind_keybinding(&mut self, action: String, cx: &mut Context) { + self.update_config(cx, |cfg| { + cfg.keybindings.insert( + action, + crate::core::config::KeybindingOverride::Exact(Vec::new()), + ); + }); + crate::ui::keymap::rebind(cx); + if let Some(s) = self.active_settings_mut() { + s.recording = None; + s.rebinding_note = None; + } + cx.notify(); + } + pub(crate) fn reset_keybinding(&mut self, action: String, cx: &mut Context) { self.update_config(cx, |cfg| { cfg.keybindings.remove(&action); @@ -10525,6 +10553,56 @@ mod keybinding_gpui_tests { ); } + /// #901, the half that happens in the UI: Alt+1…9 belongs to vim, and the + /// only gesture in the app that looks like "take this shortcut away" used + /// to *reset* the row instead — a no-op on a row nobody had overridden, + /// so the default appeared to restore itself however many times it was + /// pressed. + #[gpui::test] + fn backspace_on_a_row_unbinds_the_action_rather_than_restoring_its_default( + cx: &mut TestAppContext, + ) { + let (app, mut vcx) = harness(cx); + let shipped = vcx + .update(|_, cx| crate::ui::keymap::effective_key("ActivateTab1", cx)) + .expect("Go to Tab 1 ships with a chord"); + + begin_capture(&app, &mut vcx, "ActivateTab1"); + vcx.simulate_keystrokes("backspace"); + wait_for_binding(&mut vcx, "ActivateTab1", serde_json::json!([])); + + vcx.update(|_, cx| { + assert_eq!( + crate::ui::keymap::effective_key("ActivateTab1", cx), + None, + "the row has no chord left to show" + ); + let typed = [gpui::Keystroke::parse(&shipped).expect("the chord parses")]; + let context = [gpui::KeyContext::parse("Terminal").expect("the context parses")]; + assert!( + cx.key_bindings() + .borrow() + .bindings_for_input(&typed, &context) + .0 + .is_empty(), + "{shipped} must reach the terminal now, not the tab switcher" + ); + }); + + // Reversible, and by the button that is already on the row: an + // overridden action — unbound counts — shows **Reset**. + app.update_in(&mut vcx, |app, _, cx| { + app.reset_keybinding("ActivateTab1".to_string(), cx) + }); + vcx.update(|_, cx| { + assert_eq!( + crate::ui::keymap::effective_key("ActivateTab1", cx).as_deref(), + Some(shipped.as_str()), + "Reset is the way back to the shipped chord" + ); + }); + } + #[gpui::test] fn escape_cancels_capture_without_writing(cx: &mut TestAppContext) { let (app, mut vcx) = harness(cx); diff --git a/src/ui/i18n/en.rs b/src/ui/i18n/en.rs index a2f97082..631a1c49 100644 --- a/src/ui/i18n/en.rs +++ b/src/ui/i18n/en.rs @@ -628,7 +628,7 @@ pub fn translate_en(key: L10nKey) -> &'static str { "tmux remaps pane/tab actions onto prefix sequences (e.g. Ctrl-B then C)." } L10nKey::SettingsPrefix => "Prefix", - L10nKey::SettingsPressKeys => "Press keys…", + L10nKey::SettingsPressKeys => "Press keys… · ⌫ for no shortcut", L10nKey::SettingsPauseToSaveEsc => "pause to save · Esc", L10nKey::SettingsKeybindingsIntroDesc => { "Click a shortcut, then press the new keys — it saves after a brief pause. Chain keys for a sequence like Ctrl-B then X. Esc cancels; Backspace removes the last key, or resets to default if pressed first." diff --git a/src/ui/i18n/ja.rs b/src/ui/i18n/ja.rs index ee452f64..17875bfb 100644 --- a/src/ui/i18n/ja.rs +++ b/src/ui/i18n/ja.rs @@ -635,7 +635,7 @@ pub fn translate_ja(key: L10nKey) -> Option<&'static str> { "tmux では、ペイン/タブの操作をプレフィックスキーの後に行います（例: Ctrl-B の後に C）" } L10nKey::SettingsPrefix => "プレフィックスキー", - L10nKey::SettingsPressKeys => "キーを入力…", + L10nKey::SettingsPressKeys => "キーを入力… · ⌫ でショートカットなし", L10nKey::SettingsPauseToSaveEsc => "一時停止して保存 · Esc", L10nKey::SettingsKeybindingsIntroDesc => { "ショートカットをクリックして新しいキーを押すと、少し間を置いて保存されます。Ctrl-B の後に X のようなシーケンスはキーを続けて入力。Esc でキャンセル、Backspace は最後のキーを削除し、最初に押すとデフォルトに戻します" diff --git a/src/ui/i18n/zh.rs b/src/ui/i18n/zh.rs index 17790008..d5aedfdb 100644 --- a/src/ui/i18n/zh.rs +++ b/src/ui/i18n/zh.rs @@ -554,7 +554,7 @@ pub fn translate_zh(key: L10nKey) -> Option<&'static str> { "tmux 预设把窗格/标签页操作映射为前缀序列（例如 Ctrl-B 后按 C）。" } L10nKey::SettingsPrefix => "前缀", - L10nKey::SettingsPressKeys => "按下按键…", + L10nKey::SettingsPressKeys => "按下按键… · ⌫ 表示不设快捷键", L10nKey::SettingsPauseToSaveEsc => "暂停以保存 · Esc", L10nKey::SettingsKeybindingsIntroDesc => { "点击某个快捷键，再按下新按键，短暂停顿后保存。连续按键可组成序列，例如 Ctrl-B 后按 X。Esc 取消；Backspace 移除最后一个按键，最先按下则重置为默认。" diff --git a/src/ui/keymap.rs b/src/ui/keymap.rs index 3cf3e14f..f7b77e86 100644 --- a/src/ui/keymap.rs +++ b/src/ui/keymap.rs @@ -2462,6 +2462,56 @@ mod gpui_tests { }); } + /// #901: Alt+1…9 is how vim switches tabs, and tty7's default was eating + /// the whole row of them. Both halves of what the reporter wanted have to + /// hold, and hold across a restart — the complaint was that the default + /// "keeps coming back". + #[gpui::test] + fn alt_digits_can_be_moved_off_the_tab_actions_for_good(cx: &mut TestAppContext) { + use gpui::Action as _; + cx.update(|cx| { + let default_chord = per_platform("secondary-1", "alt-1"); + running_on_json( + cx, + r#"{"keybindings": {"ActivateTab1": ["alt-shift-1"], + "ActivateTab2": []}}"#, + ); + assert!( + fired(cx, default_chord).is_empty(), + "a list replaces the shipped chord, so the digit reaches the shell" + ); + assert!( + fired(cx, per_platform("secondary-2", "alt-2")).is_empty(), + "and an empty list leaves the action with no chord at all" + ); + assert_eq!( + fired(cx, "alt-shift-1").first(), + Some(&ActivateTab1::name_for_type()), + "the chord asked for in its place is live" + ); + assert_eq!(effective_key("ActivateTab2", cx), None); + + // The half that reads as "老是自动恢复": whatever the app saves has + // to load back as the same thing. A `Config` serialized and read + // again is exactly what a restart does with `config.json`. + let saved = + serde_json::to_string(&**cx.global::()).expect("the config serializes"); + cx.set_global(Config( + serde_json::from_str(&saved).expect("the saved config parses"), + )); + rebind(cx); + assert!( + fired(cx, default_chord).is_empty(), + "the shipped chord must not come back across a save and reload" + ); + assert_eq!( + fired(cx, "alt-shift-1").first(), + Some(&ActivateTab1::name_for_type()), + ); + assert_eq!(effective_key("ActivateTab2", cx), None); + }); + } + #[gpui::test] fn a_chord_added_under_the_tmux_preset_joins_the_preset_chord(cx: &mut TestAppContext) { use gpui::Action as _;