starred/tty7
1
0
Fork 0
mirror of https://github.com/l0ng-ai/tty7.git synced 2026-09-21 16:02:20 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(keymap): a keybinding in config adds a chord instead of replacing the default (#868)

Browse Source 
effective_bindings kept one chord per action and set_binding overwrote that
slot, so "NextTab": "cmd-shift-]" silently took Ctrl+Tab away.

A string in keybindings now adds a chord beside the action's default (or
preset) chord; "" still unbinds, as configs and the docs already rely on; a
list is the exact chord set, [] unbinds. Configured chords are installed after
every shipped one, so a chord the user names wins a tie with another action's
default. The Settings page lists every chord of an action, and recording a
shortcut writes the list shape (it sets the binding) and takes only the stolen
chord from the action that had it.

Claude-Session: https://claude.ai/code/session_01JRqYZ9E153WpSHGS2AW3BM
This commit is contained in:
l0ng-ai
2026-09-14 21:48:19 +08:00
parent 37c2ef2176
commit 436ea04f33
6 changed files with 226 additions and 75 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/tty7-core/src/core/config.rs
+18 -1
View File
@@ -110,6 +110,23 @@ impl serde::Serialize for FontFeatures {
}
}
/// One action's line in `keybindings`.
///
/// The two shapes mean different things, so a save writes back whichever one
/// was read.
#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Deserialize, Serialize)]
#[serde(untagged)]
pub enum KeybindingOverride {
/// `"NextTab": "cmd-shift-]"` — a chord *beside* the ones the action
/// already has, the way VS Code, Zed and kitty read a line like it (#868).
/// Empty unbinds the action, which is what `""` has always meant here.
Add(String),
/// `"NextTab": ["cmd-shift-]"]` — exactly these chords, replacing the
/// default and the preset's. `[]` unbinds. This is what the Settings page
/// writes, because recording a shortcut there sets it.
Exact(Vec<String>),
}
#[derive(Debug, Clone, Deserialize, Serialize)]
#[serde(default)]
pub struct Config {
@@ -150,7 +167,7 @@ pub struct Config {
pub window_backdrop: WindowBackdrop,
#[serde(default = "default_true")]
pub dim_inactive_panes: bool,
pub keybindings: HashMap<String, String>,
pub keybindings: HashMap<String, KeybindingOverride>,
#[serde(default = "default_preset")]
pub keybinding_preset: String,
#[serde(default = "default_prefix")]
docs/customization/keybindings.mdx
+15 -2
View File
@@ -39,13 +39,26 @@ the panel tabs. They are all in the command palette, and all bindable here.
```json
{
"keybindings": {
"SplitRight": "cmd-d",
"NextTab": "cmd-shift-]",
"SplitRight": ["cmd-d"],
"ResizePaneLeft": "ctrl-alt-left",
"ToggleSftp": "cmd-shift-u"
"ToggleFullscreen": ""
}
}
```
An action's value takes one of two shapes:
| Value | Means |
|---|---|
| `"cmd-shift-]"` | **Add** this shortcut. The default keeps working — <kbd>⌃ ⇥</kbd> still switches tabs |
| `["cmd-d"]` | **Replace**: exactly these shortcuts, nothing else. List several to have several |
| `""` or `[]` | **Unbind** the action |
Recording a shortcut on the Settings page replaces, so it writes the list
shape. A shortcut you add that another action already uses wins: it is the one
that runs.
The syntax is modifiers joined by `-`, then the key. Chords are separated by a
space.
src/main.rs
+8 -8
View File
@@ -742,9 +742,10 @@ mod config_reload_tests {
}
fn bound_to_split_right(config: &mut Config, key: &str) {
config
.keybindings
.insert("SplitRight".to_string(), key.to_string());
config.keybindings.insert(
"SplitRight".to_string(),
crate::core::config::KeybindingOverride::Add(key.to_string()),
);
}
#[gpui::test]
@@ -821,11 +822,10 @@ mod config_reload_tests {
);
assert!(announced, "the breakage is announced");
assert_eq!(
cx.global::<Config>()
.keybindings
.get("SplitRight")
.map(String::as_str),
Some("ctrl-alt-9"),
cx.global::<Config>().keybindings.get("SplitRight"),
Some(&crate::core::config::KeybindingOverride::Add(
"ctrl-alt-9".to_string()
)),
"the running config survives the broken file"
);
assert_eq!(
src/ui/app.rs
+36 -21
View File
@@ -7194,16 +7194,32 @@ impl Tty7App {
// only `same_chord` sees it. Compared as text, the displacement never
// fires and both bindings survive onto that keystroke, where which one
// wins is arbitrary (#750).
let displaced = crate::ui::keymap::effective_bindings(cx)
//
// Only that chord moves: the action that had it keeps any others it
// has, since an action can carry several (#868) and emptying it would
// take away keys that were never on this keystroke. An extra default —
// Alt+Enter beside Shift+Enter — follows its action's first chord rather
// than being one of its own, so its owner is unbound outright, as it
// always was.
use crate::ui::keymap::same_chord;
let displaced: Option<(String, Vec<String>)> = crate::ui::keymap::effective_chords(cx)
.into_iter()
.chain(crate::ui::keymap::extra_bindings(cx))
.find(|(a, k)| *a != action && crate::ui::keymap::same_chord(k, &spec))
.map(|(a, _)| a);
.find(|(a, chords)| *a != action && chords.iter().any(|k| same_chord(k, &spec)))
.map(|(a, chords)| {
let rest = chords.into_iter().filter(|k| !same_chord(k, &spec));
(a, rest.collect())
})
.or_else(|| {
crate::ui::keymap::extra_bindings(cx)
.into_iter()
.find(|(a, k)| *a != action && same_chord(k, &spec))
.map(|(a, _)| (a, Vec::new()))
});
// A trailing "…" on an action name marks a command that opens
// something; it is not punctuation, and inside a sentence it reads as
// the sentence trailing off — "Rename Tab… took the shortcut from".
let in_prose = |name: &str| name.trim_end_matches('…').to_string();
let note = displaced.as_ref().map(|other| {
let note = displaced.as_ref().map(|(other, _)| {
t_fmt(
L10nKey::AppKeybindingDisplacedNote,
&[
@@ -7218,11 +7234,17 @@ impl Tty7App {
],
)
});
// Both written as lists. Recording a shortcut sets it — the row showed
// one chord and now shows another — and a bare string in config adds a
// chord beside the default instead (#868).
self.update_config(cx, |cfg| {
if let Some(other) = &displaced {
cfg.keybindings.insert(other.clone(), String::new());
use crate::core::config::KeybindingOverride;
if let Some((other, rest)) = &displaced {
cfg.keybindings
.insert(other.clone(), KeybindingOverride::Exact(rest.clone()));
}
cfg.keybindings.insert(action, spec);
cfg.keybindings
.insert(action, KeybindingOverride::Exact(vec![spec]));
});
crate::ui::keymap::rebind(cx);
if let Some(s) = self.active_settings_mut() {
@@ -10441,11 +10463,7 @@ mod keybinding_gpui_tests {
// the row showed one chord and now shows another. A bare string in
// config adds a chord beside the default (#868), which is not what
// the person at the row just did.
wait_for_binding(
&mut vcx,
"NewTab",
serde_json::json!(["secondary-shift-n"]),
);
wait_for_binding(&mut vcx, "NewTab", serde_json::json!(["secondary-shift-n"]));
let recording = app.update_in(&mut vcx, |app, _, _| {
app.active_settings().map(|s| s.recording.is_some())
@@ -10471,16 +10489,13 @@ mod keybinding_gpui_tests {
}
#[gpui::test]
fn recording_a_chord_another_action_also_has_takes_only_that_chord(
cx: &mut TestAppContext,
) {
fn recording_a_chord_another_action_also_has_takes_only_that_chord(cx: &mut TestAppContext) {
let (app, mut vcx) = harness(cx);
vcx.update(|_, cx| {
cx.global_mut::<Config>().keybindings =
serde_json::from_value(serde_json::json!({
"NextTab": ["ctrl-tab", "secondary-alt-n"],
}))
.expect("the binding loads");
cx.global_mut::<Config>().keybindings = serde_json::from_value(serde_json::json!({
"NextTab": ["ctrl-tab", "secondary-alt-n"],
}))
.expect("the binding loads");
crate::ui::keymap::rebind(cx);
});
begin_capture(&app, &mut vcx, "NewTab");
src/ui/keymap.rs
+129 -39
View File
@@ -1,7 +1,7 @@
use gpui::{App, Global, KeyBinding, Keystroke, NoAction};
use crate::core::actions::*;
use crate::core::config::Config;
use crate::core::config::{Config, KeybindingOverride};
use crate::terminal::view::{
AlternatePaste, ClearScrollback, CopyText, FindInTerminal, FindNext, FindPrevious,
InsertNewline, InsertNewlineFallback, PasteText,
@@ -106,9 +106,9 @@ pub fn rebind(cx: &mut App) {
/// own `save()`, which fires on a sidebar drag — so it compares the triple
/// before and after and only rebinds when one of these actually moved;
/// otherwise each save would rebuild the keymap for nothing (#548).
pub(crate) fn keybinding_config(cx: &App) -> (Vec<(String, String)>, String, String) {
pub(crate) fn keybinding_config(cx: &App) -> (Vec<(String, KeybindingOverride)>, String, String) {
let cfg = cx.global::<Config>();
let mut overrides: Vec<(String, String)> = cfg
let mut overrides: Vec<(String, KeybindingOverride)> = cfg
.keybindings
.iter()
.map(|(a, k)| (a.clone(), k.clone()))
@@ -881,28 +881,114 @@ fn authored_entry(action: &str) -> Option<(CommandGroup, String)> {
})
}
pub(crate) fn effective_bindings(cx: &App) -> Vec<(String, String)> {
let cfg = cx.global::<Config>();
let mut effective: Vec<(String, String)> = default_bindings()
.into_iter()
.map(|(a, k)| (a.to_string(), k.to_string()))
.collect();
for (action, key) in preset_bindings(&cfg.keybinding_preset, &cfg.prefix) {
set_binding(&mut effective, &action, key);
}
for (action, key) in &cfg.keybindings {
set_binding(&mut effective, action, key.clone());
}
effective
/// One action and the chords it answers to, split by where they came from:
/// the ones it ships with — its default, or the preset's in its place — and
/// the ones `config.json` names.
///
/// Kept apart because they install apart. gpui resolves two bindings on one
/// chord to the one added last, and a chord the user asked for by name has to
/// be that one, wherever its action sits in the table.
struct ActionChords {
action: String,
shipped: Vec<String>,
configured: Vec<String>,
}
fn set_binding(effective: &mut [(String, String)], action: &str, key: String) {
match effective.iter_mut().find(|(a, _)| a == action) {
Some(slot) => slot.1 = key,
// A hand-edited config.json with a typo used to vanish into this
// branch. The Keybindings page lists every name that works.
None => log::warn!("keybinding for unknown action {action:?} ignored"),
fn resolve_chords(
preset: &str,
prefix: &str,
overrides: &std::collections::HashMap<String, KeybindingOverride>,
) -> Vec<ActionChords> {
let mut resolved: Vec<ActionChords> = default_bindings()
.into_iter()
.map(|(action, key)| ActionChords {
action: action.to_string(),
shipped: [key]
.into_iter()
.filter(|k| !k.is_empty())
.map(str::to_string)
.collect(),
configured: Vec::new(),
})
.collect();
// A preset is a scheme rather than an addition: its chord takes the
// default's place.
for (action, key) in preset_bindings(preset, prefix) {
if let Some(slot) = resolved.iter_mut().find(|s| s.action == action) {
slot.shipped = vec![key];
}
}
for (action, value) in overrides {
let Some(slot) = resolved.iter_mut().find(|s| s.action == *action) else {
// A hand-edited config.json with a typo used to vanish into this
// branch. The Keybindings page lists every name that works.
log::warn!("keybinding for unknown action {action:?} ignored");
continue;
};
match value {
// `""` has always been how a config retires a chord — the docs
// spell `"AlternatePaste": ""`, and Settings wrote it for an action
// whose chord was taken — so it goes on unbinding the action.
KeybindingOverride::Add(key) if key.is_empty() => {
slot.shipped.clear();
}
// A chord beside the ones the action has, not in place of them
// (#868): `"NextTab": "cmd-shift-]"` is a second way to switch tabs,
// and Ctrl+Tab going dead because of it was the bug.
KeybindingOverride::Add(key) => {
if !slot.shipped.iter().any(|k| same_chord(k, key)) {
slot.configured.push(key.clone());
}
}
KeybindingOverride::Exact(keys) => {
slot.shipped.clear();
for key in keys {
if !key.is_empty() && !slot.configured.iter().any(|k| same_chord(k, key)) {
slot.configured.push(key.clone());
}
}
}
}
}
resolved
}
/// Every chord as an `(action, chord)` pair, in the order the keymap is built
/// from: all the shipped chords, then all the configured ones, so that a
/// configured chord landing on another action's default is the binding that
/// wins it. An action with no chord at all has no pair.
fn flatten_chords(resolved: &[ActionChords]) -> Vec<(String, String)> {
let pairs = |pick: fn(&ActionChords) -> &Vec<String>| {
resolved
.iter()
.flat_map(move |s| pick(s).iter().map(|k| (s.action.clone(), k.clone())))
};
pairs(|s| &s.shipped)
.chain(pairs(|s| &s.configured))
.collect()
}
pub(crate) fn effective_bindings(cx: &App) -> Vec<(String, String)> {
let cfg = cx.global::<Config>();
flatten_chords(&resolve_chords(
&cfg.keybinding_preset,
&cfg.prefix,
&cfg.keybindings,
))
}
/// Every action in table order with all of its chords, shipped first — an
/// empty list for an action that has none. What a page listing the actions
/// reads; the keymap reads [`effective_bindings`].
pub(crate) fn effective_chords(cx: &App) -> Vec<(String, Vec<String>)> {
let cfg = cx.global::<Config>();
resolve_chords(&cfg.keybinding_preset, &cfg.prefix, &cfg.keybindings)
.into_iter()
.map(|mut s| {
s.shipped.append(&mut s.configured);
(s.action, s.shipped)
})
.collect()
}
fn preset_bindings(preset: &str, prefix: &str) -> Vec<(String, String)> {
@@ -1726,14 +1812,16 @@ mod tests {
// one line in a `config.json`, so both are asserted against the whole
// default table with that line applied — a bare one-entry table would
// pass either assertion without the escape hatch working at all.
let mut retired = effective.clone();
set_binding(&mut retired, "AlternatePaste", String::new());
let with = |action: &str, value: KeybindingOverride| {
let overrides = std::collections::HashMap::from([(action.to_string(), value)]);
flatten_chords(&resolve_chords("default", "ctrl-b", &overrides))
};
let retired = with("AlternatePaste", KeybindingOverride::Add(String::new()));
assert!(
dispatched(&retired, "ctrl-v", "Terminal").is_empty(),
"an emptied AlternatePaste gives Ctrl+V back to the shell"
);
let mut everywhere = effective.clone();
set_binding(&mut everywhere, "PasteText", "ctrl-v".to_string());
let everywhere = with("PasteText", KeybindingOverride::Add("ctrl-v".to_string()));
for context in ["Terminal", "Terminal alt_screen"] {
assert!(
dispatched(&everywhere, "ctrl-v", context).contains(&PasteText::name_for_type()),
@@ -2200,23 +2288,25 @@ mod gpui_tests {
{
let cfg = cx.global_mut::<Config>();
cfg.keybinding_preset = "tmux".to_string();
cfg.keybindings
.insert("NewTab".to_string(), "secondary-shift-n".to_string());
cfg.keybindings.insert(
"NewTab".to_string(),
KeybindingOverride::Add("secondary-shift-n".to_string()),
);
}
rebind(cx);
let eff = effective_bindings(cx);
let key_of = |action: &str| {
let keys_of = |action: &str| {
eff.iter()
.find(|(a, _)| a == action)
.filter(|(a, _)| a == action)
.map(|(_, k)| k.clone())
.unwrap()
.collect::<Vec<_>>()
};
assert_eq!(key_of("NewTab"), "secondary-shift-n");
assert_eq!(key_of("SplitRight"), "ctrl-b %");
assert_eq!(keys_of("NewTab"), ["ctrl-b c", "secondary-shift-n"]);
assert_eq!(keys_of("SplitRight"), ["ctrl-b %"]);
assert_eq!(
key_of("TogglePalette"),
per_platform("secondary-p", "secondary-shift-p")
keys_of("TogglePalette"),
[per_platform("secondary-p", "secondary-shift-p")]
);
cx.global_mut::<Config>().keybinding_preset = "default".to_string();
@@ -2251,9 +2341,10 @@ mod gpui_tests {
assert_eq!(keybinding_config(cx), before);
// A real binding edit moves it.
cx.global_mut::<Config>()
.keybindings
.insert("RenameTab".to_string(), "ctrl-shift-r".to_string());
cx.global_mut::<Config>().keybindings.insert(
"RenameTab".to_string(),
KeybindingOverride::Add("ctrl-shift-r".to_string()),
);
assert_ne!(keybinding_config(cx), before);
// So does the preset, and the prefix.
@@ -2280,7 +2371,6 @@ mod gpui_tests {
/// What the live keymap dispatches for `keys` typed in a terminal, best
/// match first — the first entry is the action a real keypress runs.
fn fired(cx: &gpui::App, keys: &str) -> Vec<&'static str> {
use gpui::Action as _;
let input: Vec<Keystroke> = keys
.split(' ')
.map(|k| Keystroke::parse(k).expect("the typed keystroke parses"))
src/ui/settings.rs
+20 -4
View File
@@ -7200,7 +7200,7 @@ impl Tty7App {
)
};
let tmux = preset == "tmux";
let effective = crate::ui::keymap::effective_bindings(cx);
let effective = crate::ui::keymap::effective_chords(cx);
let recording = self
.active_settings()
@@ -7308,7 +7308,7 @@ impl Tty7App {
let filtering = !query.is_empty() && section_match_count(section, &query) > 0;
let mut grouped: Vec<(
crate::ui::palette::CommandGroup,
Vec<(String, String, String)>,
Vec<(String, Vec<String>, String)>,
)> = Vec::new();
for (action, key) in effective {
if filtering && !keybinding_matches_query(&action, &query) {
@@ -7330,7 +7330,7 @@ impl Tty7App {
.position(|o| o == g)
.unwrap_or(usize::MAX)
});
let rows: Vec<(String, String, String)> = grouped
let rows: Vec<(String, Vec<String>, String)> = grouped
.iter()
.flat_map(|(_, rows)| rows.iter().cloned())
.collect();
@@ -7414,7 +7414,23 @@ impl Tty7App {
.child("—")
.into_any_element()
} else {
keycaps(&key).into_any_element()
// An action can answer to more than one chord — its default and
// one added beside it in config.json (#868) — and this is the
// one page that lists them, so a row shows every one.
h_flex()
.flex_wrap()
.items_center()
.gap_2()
.children(key.iter().enumerate().map(|(n, spec)| {
h_flex()
.items_center()
.gap_2()
.when(n > 0, |d| {
d.child(div().text_xs().text_color(muted).child("/"))
})
.child(keycaps(spec))
}))
.into_any_element()
};
let action_for_click = action.clone();