test(keymap): ask the keymap what is bound, and cover the reload that binds it (#582)

The keymap tests asserted against `bound_keystrokes`, a `#[cfg(test)]` mirror
of `action_bindings` that #571 left behind when it removed the `NoAction`
retire loop — the last production reader of it. A mirror is a second
implementation of the table, so those tests could agree with it while the app
bound something else.

Each one now builds a `gpui::Keymap` from `action_bindings` and performs the
lookup gpui performs on a keypress: typing this chord in that context runs that
action. The paste test gains the other half it never asserted — that a terminal
chord does nothing outside a terminal — and the context test is renamed for
what it now checks. `bound_keystrokes` is deleted.

The watcher had no coverage at all, so the one thing #548 is for — hand-editing
config.json and having the new chord fire without a restart — was pinned
nowhere. Its tick moves out of the closure into `apply_reloaded_config`: same
statements in the same order, taking the load result as an argument and
returning whether it rebound, which the watcher ignores. Three tests drive it
against a live keymap — a reload that binds Ctrl+Alt+9 to SplitRight makes that
chord dispatch it, a reload that moves no binding does not rebuild the map, and
a quarantined reload rebinds nothing, because the global config is deliberately
not replaced on that path and the user's keys have to survive a typo.
This commit is contained in:
l0ng-ai
2026-08-13 11:49:42 +08:00
parent 664b766698
commit f3f68ead51
2 changed files with 299 additions and 83 deletions
+236 -53
View File
@@ -84,65 +84,78 @@ fn spawn_config_watcher(cx: &mut App) {
while rx.try_recv().is_ok() {}
cx.update(|cx| {
let (config, outcome) = Config::load_with_outcome();
if outcome.failed() {
// Keep the settings the app is running on: swapping the
// stand-in defaults in would flash the whole UI onto
// defaults, and the load already parked the broken file
// beside the original. It reloads itself the moment the
// file parses again.
if !announced {
announced = true;
notify_config_load_failed(cx, outcome, false);
}
// The theme files this same watcher covers must keep
// hot-reloading: a typo in config.json is no reason for
// theme editing to go dead until the app restarts. They
// read the global config, which is deliberately still the
// one the app is running on.
reload_themes(cx);
cx.refresh_windows();
return;
}
announced = false;
// The keymap is rebuilt only when a binding actually moved.
// This watcher fires for every write under the config dir —
// including the app's own `save()`, which a sidebar drag or a
// palette open triggers — so reloading bindings on each tick
// would rebuild the whole keymap for nothing, and (before
// `rebind` cleared first) leak a full table per tick (#548).
// Read past the early return above: a load that failed leaves
// the global alone, so there is nothing to compare and the
// user's keys stay in the keymap the app is dispatching on.
let keymap_before = crate::ui::keymap::keybinding_config(cx);
crate::ui::i18n::set_locale(&config.gui_language);
cx.set_global(config);
reload_themes(cx);
crate::ui::theme::apply_cursor_hide_mode(cx);
// The menu bar is built once from the current locale, so editing
// gui_language by hand has to rebuild it the same way the
// in-app language picker does.
crate::ui::theme::set_menus(cx);
crate::ui::windows::WindowRegistry::refresh_locale(cx, None);
// `custom_shells` is only ever hand-edited, so this file is the
// one place it can change from — and the inventory that carries
// it to the new-tab menu is cached per window.
crate::ui::windows::WindowRegistry::refresh_shells(cx);
// A hand-edited keybinding shows up in the settings list off the
// live global immediately; without this it never reaches the
// keymap gpui actually dispatches against, so the key looks
// bound and does nothing until restart. Gated on the triple so
// an unrelated save does not churn it.
if crate::ui::keymap::keybinding_config(cx) != keymap_before {
crate::ui::keymap::rebind(cx);
}
cx.refresh_windows();
apply_reloaded_config(cx, Config::load_with_outcome(), &mut announced);
});
}
})
.detach();
}
/// One watcher tick, once the debounce is out: what a reload does to the
/// running app.
///
/// `announced` is the one-toast-per-breakage latch, which lives across ticks
/// in the watcher. Returns whether the keymap was rebuilt — the watcher has no
/// use for that; it is how a test asks which branch ran.
fn apply_reloaded_config(
cx: &mut App,
load: (Config, crate::core::config::LoadOutcome),
announced: &mut bool,
) -> bool {
let (config, outcome) = load;
if outcome.failed() {
// Keep the settings the app is running on: swapping the stand-in
// defaults in would flash the whole UI onto defaults, and the load
// already parked the broken file beside the original. It reloads
// itself the moment the file parses again.
if !*announced {
*announced = true;
notify_config_load_failed(cx, outcome, false);
}
// The theme files this same watcher covers must keep hot-reloading: a
// typo in config.json is no reason for theme editing to go dead until
// the app restarts. They read the global config, which is deliberately
// still the one the app is running on.
reload_themes(cx);
cx.refresh_windows();
return false;
}
*announced = false;
// The keymap is rebuilt only when a binding actually moved. This watcher
// fires for every write under the config dir — including the app's own
// `save()`, which a sidebar drag or a palette open triggers — so reloading
// bindings on each tick would rebuild the whole keymap for nothing, and
// (before `rebind` cleared first) leak a full table per tick (#548).
// Read past the early return above: a load that failed leaves the global
// alone, so there is nothing to compare and the user's keys stay in the
// keymap the app is dispatching on.
let keymap_before = crate::ui::keymap::keybinding_config(cx);
crate::ui::i18n::set_locale(&config.gui_language);
cx.set_global(config);
reload_themes(cx);
crate::ui::theme::apply_cursor_hide_mode(cx);
// The menu bar is built once from the current locale, so editing
// gui_language by hand has to rebuild it the same way the in-app language
// picker does.
crate::ui::theme::set_menus(cx);
crate::ui::windows::WindowRegistry::refresh_locale(cx, None);
// `custom_shells` is only ever hand-edited, so this file is the one place
// it can change from — and the inventory that carries it to the new-tab
// menu is cached per window.
crate::ui::windows::WindowRegistry::refresh_shells(cx);
// A hand-edited keybinding shows up in the settings list off the live
// global immediately; without this it never reaches the keymap gpui
// actually dispatches against, so the key looks bound and does nothing
// until restart. Gated on the triple so an unrelated save does not churn
// it.
let rebound = crate::ui::keymap::keybinding_config(cx) != keymap_before;
if rebound {
crate::ui::keymap::rebind(cx);
}
cx.refresh_windows();
rebound
}
fn is_theme_file(p: &std::path::Path) -> bool {
p.parent().and_then(|d| d.file_name()) == Some(std::ffi::OsStr::new("themes"))
&& p.extension().and_then(|e| e.to_str()).is_some_and(|e| {
@@ -556,6 +569,176 @@ fn main() {
});
}
/// The watcher tick, from a reloaded file to the keys the app dispatches on.
///
/// The one thing #548 is for — hand-editing config.json and having the new
/// chord fire without a restart — crosses a file watcher, a debounce and a
/// keymap rebuild, and used to be covered nowhere. These drive
/// `apply_reloaded_config` directly, which is the whole body of the watcher's
/// tick, so the reload path is exercised without waiting on the filesystem.
#[cfg(test)]
mod config_reload_tests {
use super::{apply_reloaded_config, is_theme_file};
use crate::core::actions::SplitRight;
use crate::core::config::{Config, LoadOutcome};
use gpui::{Action as _, App, KeyContext, Keystroke, TestAppContext};
/// What the live keymap — the one gpui dispatches against — does with
/// `keys` typed in a terminal.
fn dispatched(cx: &App, keys: &str) -> Vec<&'static str> {
let typed = [Keystroke::parse(keys).expect("the typed keystroke parses")];
let context = [KeyContext::parse("Terminal").expect("the context parses")];
cx.key_bindings()
.borrow()
.bindings_for_input(&typed, &context)
.0
.iter()
.map(|b| b.action().name())
.collect()
}
/// An app running on `config`, as far as the config watcher can tell:
/// the globals its tick reads, and a keymap built from that config.
fn running_on(cx: &mut App, config: Config) {
let dir = std::env::temp_dir().join(format!("tty7-reload-{}", std::process::id()));
std::fs::create_dir_all(&dir).ok();
crate::core::config::set_config_dir(dir);
gpui_component::init(cx);
cx.set_global(config);
crate::ui::windows::WindowRegistry::init(cx);
crate::ui::presets::load_registry(cx);
crate::ui::keymap::init(cx);
}
fn bound_to_split_right(config: &mut Config, key: &str) {
config
.keybindings
.insert("SplitRight".to_string(), key.to_string());
}
#[gpui::test]
fn a_hand_edited_binding_fires_without_a_restart(cx: &mut TestAppContext) {
cx.update(|cx| {
running_on(cx, Config::default());
assert!(
dispatched(cx, "ctrl-alt-9").is_empty(),
"nothing is on the chord before the edit"
);
let mut edited = Config::default();
bound_to_split_right(&mut edited, "ctrl-alt-9");
let mut announced = false;
assert!(
apply_reloaded_config(cx, (edited, LoadOutcome::Parsed), &mut announced),
"a moved binding rebuilds the keymap"
);
assert_eq!(
dispatched(cx, "ctrl-alt-9"),
vec![SplitRight::name_for_type()],
"the hand-edited chord reaches the keymap gpui dispatches on"
);
});
}
#[gpui::test]
fn a_save_that_moves_no_binding_leaves_the_keymap_alone(cx: &mut TestAppContext) {
cx.update(|cx| {
running_on(cx, Config::default());
// The app's own `save()` fires this watcher on every sidebar drag.
let mut unrelated = Config::default();
unrelated.dim_inactive_panes = !unrelated.dim_inactive_panes;
let mut announced = false;
assert!(
!apply_reloaded_config(
cx,
(unrelated.clone(), LoadOutcome::Parsed),
&mut announced
),
"an unrelated save must not rebuild the keymap"
);
assert_eq!(
cx.global::<Config>().dim_inactive_panes,
unrelated.dim_inactive_panes,
"the reload still took effect; only the rebind was skipped"
);
});
}
#[gpui::test]
fn a_broken_config_does_not_take_the_users_keys_away(cx: &mut TestAppContext) {
cx.update(|cx| {
let mut user = Config::default();
bound_to_split_right(&mut user, "ctrl-alt-9");
running_on(cx, user);
assert_eq!(
dispatched(cx, "ctrl-alt-9"),
vec![SplitRight::name_for_type()]
);
// A quarantined load hands back stand-in defaults, and the global
// is deliberately left alone — so nothing may rebind off them.
let mut announced = false;
assert!(
!apply_reloaded_config(
cx,
(Config::default(), LoadOutcome::Quarantined),
&mut announced
),
"a load that failed has nothing to compare and must not rebind"
);
assert!(announced, "the breakage is announced");
assert_eq!(
cx.global::<Config>()
.keybindings
.get("SplitRight")
.map(String::as_str),
Some("ctrl-alt-9"),
"the running config survives the broken file"
);
assert_eq!(
dispatched(cx, "ctrl-alt-9"),
vec![SplitRight::name_for_type()],
"and so do the keys the app is dispatching on"
);
// The file parses again: the latch clears, so a second breakage
// can speak up.
let mut fixed = Config::default();
bound_to_split_right(&mut fixed, "ctrl-alt-8");
assert!(apply_reloaded_config(
cx,
(fixed, LoadOutcome::Parsed),
&mut announced
));
assert!(!announced);
assert_eq!(
dispatched(cx, "ctrl-alt-8"),
vec![SplitRight::name_for_type()]
);
assert!(
dispatched(cx, "ctrl-alt-9").is_empty(),
"the chord the fixed file dropped is retired"
);
});
}
/// The watcher covers the whole config directory, and the themes half of
/// it has to keep reloading even when config.json does not parse.
#[test]
fn only_theme_files_beside_the_config_count_as_themes() {
use std::path::Path;
assert!(is_theme_file(Path::new("/cfg/themes/solar.yaml")));
assert!(is_theme_file(Path::new("/cfg/themes/solar.YML")));
assert!(is_theme_file(Path::new("/cfg/themes/solar.itermcolors")));
assert!(!is_theme_file(Path::new("/cfg/themes/notes.txt")));
assert!(!is_theme_file(Path::new("/cfg/config.json")));
assert!(!is_theme_file(Path::new("/cfg/solar.yaml")));
}
}
#[cfg(all(test, unix))]
mod tests {
use super::merge_paths;
+63 -30
View File
@@ -183,22 +183,6 @@ fn action_bindings(effective: &[(String, String)]) -> Vec<KeyBinding> {
bindings
}
/// The keystroke and context of every binding `action_bindings` installs.
/// The rebind used to keep this in a global to retire the previous set one
/// `NoAction` at a time; a rebuild replaces the whole map instead, so this is
/// now only the tests' way of asking what a config would install.
#[cfg(test)]
fn bound_keystrokes(effective: &[(String, String)]) -> Vec<(String, Option<&'static str>)> {
let extras = extra_keystrokes(effective);
effective
.iter()
.map(|(a, k)| (a.as_str(), k.as_str()))
.chain(extras.iter().map(|(a, k)| (*a, *k)))
.filter(|(_, k)| !k.is_empty() && keystroke_is_valid(k))
.map(|(a, k)| (k.to_string(), action_context(a)))
.collect()
}
fn per_platform(mac: &'static str, other: &'static str) -> &'static str {
if cfg!(target_os = "macos") {
mac
@@ -1057,6 +1041,30 @@ fn make_binding(action: &str, keystroke: &str) -> Option<KeyBinding> {
#[cfg(test)]
mod tests {
use super::*;
use gpui::Action as _;
/// The actions a keymap built from `action_bindings` dispatches for `keys`
/// typed in `context`, in precedence order — the same lookup gpui performs
/// on a real keypress.
///
/// Bindings are asserted through this rather than through a mirror of the
/// table, so a chord the app would not really install, or would install in
/// another context, cannot pass.
fn dispatched(effective: &[(String, String)], keys: &str, context: &str) -> Vec<&'static str> {
let mut keymap = gpui::Keymap::default();
keymap.add_bindings(action_bindings(effective));
let input: Vec<Keystroke> = keys
.split(' ')
.map(|k| Keystroke::parse(k).expect("the typed keystroke parses"))
.collect();
let context = [gpui::KeyContext::parse(context).expect("the context parses")];
keymap
.bindings_for_input(&input, &context)
.0
.iter()
.map(|b| b.action().name())
.collect()
}
#[test]
fn every_dispatchable_action_has_a_slot_to_bind_it_in() {
@@ -1226,10 +1234,9 @@ mod tests {
assert!(extra_keystrokes(&effective).contains(&("InsertNewline", "alt-enter")));
for key in ["shift-enter", "alt-enter"] {
assert!(keystroke_is_valid(key), "{key} does not parse");
assert!(make_binding("InsertNewline", key).is_some());
assert!(
bound_keystrokes(&effective).iter().any(|(k, _)| k == key),
"{key} is not remembered as installed"
dispatched(&effective, key, "Terminal").contains(&InsertNewline::name_for_type()),
"{key} does not reach InsertNewline in a terminal"
);
}
assert_eq!(key_tokens("shift-enter"), vec![SHIFT, ""]);
@@ -1313,10 +1320,12 @@ mod tests {
assert!(extra_keystrokes(&effective).contains(&("PasteText", "shift-insert")));
for key in ["ctrl-shift-v", "shift-insert"] {
assert!(
bound_keystrokes(&effective)
.iter()
.any(|(k, c)| k == key && *c == Some("Terminal")),
"{key} must be installed in the Terminal context and remembered for rebind"
dispatched(&effective, key, "Terminal").contains(&PasteText::name_for_type()),
"{key} must paste in a terminal"
);
assert!(
!dispatched(&effective, key, "Workspace").contains(&PasteText::name_for_type()),
"{key} is a terminal chord and must not paste outside one"
);
}
let rebound = vec![("PasteText".to_string(), "ctrl-alt-v".to_string())];
@@ -1333,9 +1342,16 @@ mod tests {
let effective = vec![("InsertNewline".to_string(), "ctrl-o".to_string())];
assert!(extra_keystrokes(&effective).is_empty());
assert_eq!(
bound_keystrokes(&effective),
vec![("ctrl-o".to_string(), Some("Terminal"))]
dispatched(&effective, "ctrl-o", "Terminal"),
vec![InsertNewline::name_for_type()],
"the chord the config asks for is the one the keymap dispatches"
);
for retired in ["shift-enter", "alt-enter"] {
assert!(
dispatched(&effective, retired, "Terminal").is_empty(),
"{retired} is off the action now and must dispatch nothing"
);
}
let unbound = vec![("InsertNewline".to_string(), String::new())];
assert!(extra_keystrokes(&unbound).is_empty());
@@ -1343,19 +1359,36 @@ mod tests {
}
#[test]
fn bound_keystrokes_remember_the_context_each_binding_was_installed_in() {
fn each_binding_lands_in_the_context_its_action_is_scoped_to() {
let effective = vec![
("InsertNewline".to_string(), "shift-enter".to_string()),
("NewTab".to_string(), "secondary-t".to_string()),
];
// The newline is a terminal chord, and its default ships the fallback
// beside it; both are scoped, so neither reaches the rest of the app.
let mut newline = dispatched(&effective, "shift-enter", "Terminal");
newline.sort_unstable();
assert_eq!(
bound_keystrokes(&effective),
newline,
vec![
("shift-enter".to_string(), Some("Terminal")),
("secondary-t".to_string(), None),
("alt-enter".to_string(), Some("Terminal")),
InsertNewline::name_for_type(),
InsertNewlineFallback::name_for_type(),
]
);
assert!(dispatched(&effective, "shift-enter", "Workspace").is_empty());
assert_eq!(
dispatched(&effective, "alt-enter", "Terminal"),
vec![InsertNewline::name_for_type()],
"the extra chord follows its action's scope"
);
// A window action has no context: it has to work from a terminal too.
for context in ["Terminal", "Workspace"] {
assert_eq!(
dispatched(&effective, "secondary-t", context),
vec![NewTab::name_for_type()],
"NewTab must not be scoped to {context}"
);
}
}
#[test]