From 088922dfabca152047c8f39f660893b3b19dbe2d Mon Sep 17 00:00:00 2001 From: Ogulcan Celik Date: Mon, 22 Jun 2026 23:36:43 +0300 Subject: [PATCH] fix: let user keybinds displace defaults refs #747 --- src/app/mod.rs | 42 ++- src/config.rs | 170 ++++++++- src/config/keybinds.rs | 777 +++++++++++++++++++++++++++++------------ src/config/model.rs | 342 +++++++++++++++++- src/server/headless.rs | 10 +- 5 files changed, 1095 insertions(+), 246 deletions(-) diff --git a/src/app/mod.rs b/src/app/mod.rs index 4fd94697..bf4fecca 100644 --- a/src/app/mod.rs +++ b/src/app/mod.rs @@ -1272,11 +1272,12 @@ impl App { |section: &str| invalid_sections.iter().any(|invalid| invalid == section); if !invalid_section("keys") { - match config.live_keybinds() { - Ok(live) => { + match config.live_keybinds_with_diagnostics() { + Ok((live, keybind_diagnostics)) => { self.state.prefix_code = live.prefix.0; self.state.prefix_mods = live.prefix.1; self.state.keybinds = live.keybinds; + diagnostics.extend(keybind_diagnostics); } Err(keybind_diagnostics) => { diagnostics.extend( @@ -2617,7 +2618,7 @@ mod tests { } #[test] - fn reload_config_keeps_current_keybinds_on_invalid_binding_but_applies_other_sections() { + fn reload_config_disables_invalid_binding_but_applies_valid_keymap_and_other_sections() { let _guard = config_env_lock().lock().unwrap(); let path = temp_config_path("reload-config-invalid-keybind"); std::fs::create_dir_all(path.parent().unwrap()).unwrap(); @@ -2630,7 +2631,6 @@ mod tests { let mut app = test_app(); let original_prefix = (app.state.prefix_code, app.state.prefix_mods); - let original_keybinds = app.state.keybinds.new_workspace.clone(); let report = app.reload_config(); assert_eq!(report.status, crate::config::ConfigReloadStatus::Partial); @@ -2638,7 +2638,7 @@ mod tests { (app.state.prefix_code, app.state.prefix_mods), original_prefix ); - assert_eq!(app.state.keybinds.new_workspace, original_keybinds); + assert!(app.state.keybinds.new_workspace.bindings.is_empty()); assert_eq!( app.state.toast_config.delivery, crate::config::ToastDelivery::Terminal @@ -2648,13 +2648,43 @@ mod tests { .config_diagnostic .as_deref() .is_some_and(|message| { - message.contains("keys.new_workspace") && message.contains("kept current keybinds") + message.contains("keys.new_workspace") && message.contains("disabling binding") })); std::env::remove_var(crate::config::CONFIG_PATH_ENV_VAR); let _ = std::fs::remove_dir_all(path.parent().unwrap()); } + #[test] + fn reload_config_user_binding_displaces_default_without_rejecting_prefix() { + let _guard = config_env_lock().lock().unwrap(); + let path = temp_config_path("reload-config-user-binding-displaces-default"); + std::fs::create_dir_all(path.parent().unwrap()).unwrap(); + std::fs::write( + &path, + "[keys]\nprefix = \"ctrl+space\"\nprevious_workspace = \"prefix+shift+l\"\n", + ) + .unwrap(); + std::env::set_var(crate::config::CONFIG_PATH_ENV_VAR, &path); + + let mut app = test_app(); + let report = app.reload_config(); + + assert_eq!(report.status, crate::config::ConfigReloadStatus::Applied); + assert_eq!(app.state.prefix_code, KeyCode::Char(' ')); + assert_eq!(app.state.prefix_mods, KeyModifiers::CONTROL); + assert!(app + .state + .keybinds + .previous_workspace + .matches_prefix(&KeyEvent::new(KeyCode::Char('l'), KeyModifiers::SHIFT))); + assert!(app.state.keybinds.swap_pane_right.bindings.is_empty()); + assert!(app.state.config_diagnostic.is_none()); + + std::env::remove_var(crate::config::CONFIG_PATH_ENV_VAR); + let _ = std::fs::remove_dir_all(path.parent().unwrap()); + } + #[test] fn reload_config_preserves_invalid_ui_section_but_applies_valid_keys() { let _guard = config_env_lock().lock().unwrap(); diff --git a/src/config.rs b/src/config.rs index 003178ab..12e9cf1c 100644 --- a/src/config.rs +++ b/src/config.rs @@ -19,9 +19,9 @@ pub use self::{ }, model::{ validated_sidebar_bounds, AgentPanelSortConfig, Config, ConfigReloadReport, - ConfigReloadStatus, KeysConfig, NewTerminalCwdConfig, ShellModeConfig, - ToastClipboardPosition, ToastConfig, ToastDelivery, ToastHerdrPosition, - UpdateChannelConfig, MAX_TOAST_DELAY_SECONDS, + ConfigReloadStatus, NewTerminalCwdConfig, ShellModeConfig, ToastClipboardPosition, + ToastConfig, ToastDelivery, ToastHerdrPosition, UpdateChannelConfig, + MAX_TOAST_DELAY_SECONDS, }, sound::SoundConfig, theme::{parse_color, CustomThemeColors, ThemeConfig}, @@ -80,26 +80,32 @@ impl Config { }) } + #[cfg(test)] pub fn live_keybinds(&self) -> Result> { + self.live_keybinds_with_diagnostics() + .map(|(live, _diagnostics)| live) + } + + pub(crate) fn live_keybinds_with_diagnostics( + &self, + ) -> Result<(LiveKeybindConfig, Vec), Vec> { let (prefix_diag, prefix, keybind_diags, keybinds) = self.validated_keybinds(); - let diagnostics: Vec = prefix_diag.into_iter().chain(keybind_diags).collect(); - if diagnostics.is_empty() { - Ok(LiveKeybindConfig { prefix, keybinds }) + if let Some(prefix_diag) = prefix_diag { + Err(std::iter::once(prefix_diag).chain(keybind_diags).collect()) } else { - Err(diagnostics) + Ok((LiveKeybindConfig { prefix, keybinds }, keybind_diags)) } } pub(crate) fn local_keybindings_profile_toml(&self) -> Result { - let mut keys = self.keys.clone(); - keys.command.clear(); - #[derive(serde::Serialize)] struct KeysProfile { - keys: KeysConfig, + keys: model::KeysConfigOverlay, } - toml::to_string_pretty(&KeysProfile { keys }) + toml::to_string_pretty(&KeysProfile { + keys: self.keys.local_profile(&self.keybinds()), + }) } } @@ -132,6 +138,146 @@ command = "lazygit" assert!(!profile.contains("[[keys.command]]")); } + #[test] + fn local_keybindings_profile_preserves_user_default_provenance() { + let config: Config = toml::from_str( + r#" +[keys] +zoom = "prefix+?" +"#, + ) + .unwrap(); + + let profile = config.local_keybindings_profile_toml().unwrap(); + let round_tripped: Config = toml::from_str(&profile).unwrap(); + + assert!(profile.contains("zoom = \"prefix+?\"")); + assert!(!profile.contains("help = \"prefix+?\"")); + assert!(round_tripped + .keybinds() + .zoom + .bindings + .iter() + .any(|binding| binding.label == "prefix+?")); + assert!(round_tripped.keybinds().help.bindings.is_empty()); + } + + #[test] + fn local_keybindings_profile_omits_default_displaced_by_user_prefix() { + let config: Config = toml::from_str( + r#" +[keys] +prefix = "n" +"#, + ) + .unwrap(); + + let profile = config.local_keybindings_profile_toml().unwrap(); + let round_tripped: Config = toml::from_str(&profile).unwrap(); + + assert!(profile.contains("prefix = \"n\"")); + assert!(!profile.contains("next_tab = \"prefix+n\"")); + assert!(round_tripped.keybinds().next_tab.bindings.is_empty()); + } + + #[test] + fn local_keybindings_profile_preserves_legacy_indexed_tab_source() { + let config: Config = toml::from_str( + r#" +[keys.indexed] +tabs = "ctrl" +"#, + ) + .unwrap(); + + let profile = config.local_keybindings_profile_toml().unwrap(); + let round_tripped: Config = toml::from_str(&profile).unwrap(); + let keybinds = round_tripped.keybinds(); + let switch_tab_labels: Vec<_> = keybinds + .switch_tab + .iter() + .map(|binding| binding.label.as_str()) + .collect(); + + assert!(profile.contains("[keys.indexed]")); + assert!(profile.contains("tabs = \"ctrl\"")); + assert!(!profile.contains("switch_tab = \"prefix+1..9\"")); + assert_eq!(switch_tab_labels.len(), 9); + assert!(switch_tab_labels + .iter() + .all(|label| label.starts_with("ctrl+"))); + } + + #[test] + fn local_keybindings_profile_keeps_invalid_legacy_indexed_default_disabled() { + let config: Config = toml::from_str( + r#" +[keys.indexed] +tabs = "bogus" +"#, + ) + .unwrap(); + + let profile = config.local_keybindings_profile_toml().unwrap(); + let round_tripped: Config = toml::from_str(&profile).unwrap(); + + assert!(profile.contains("[keys.indexed]")); + assert!(profile.contains("tabs = \"bogus\"")); + assert!(!profile.contains("switch_tab = \"prefix+1..9\"")); + assert!(round_tripped.keybinds().switch_tab.is_empty()); + } + + #[test] + fn local_keybindings_profile_keeps_default_displaced_by_omitted_command_disabled() { + let config: Config = toml::from_str( + r#" +[[keys.command]] +key = "prefix+n" +command = "echo next" +"#, + ) + .unwrap(); + + let profile = config.local_keybindings_profile_toml().unwrap(); + let round_tripped: Config = toml::from_str(&profile).unwrap(); + + assert!(!profile.contains("[[keys.command]]")); + assert!(!profile.contains("command =")); + assert!(profile.contains("next_tab = \"\"")); + assert!(round_tripped.keybinds().next_tab.bindings.is_empty()); + } + + #[test] + fn local_keybindings_profile_preserves_partially_displaced_indexed_default() { + let config: Config = toml::from_str( + r#" +[[keys.command]] +key = "prefix+1" +command = "echo one" +"#, + ) + .unwrap(); + + let profile = config.local_keybindings_profile_toml().unwrap(); + let round_tripped: Config = toml::from_str(&profile).unwrap(); + let keybinds = round_tripped.keybinds(); + let switch_tab_labels: Vec<_> = keybinds + .switch_tab + .iter() + .map(|binding| binding.label.as_str()) + .collect(); + + assert!(!profile.contains("[[keys.command]]")); + assert!(!profile.contains("switch_tab = \"prefix+1..9\"")); + assert!(profile.contains("\"prefix+2\"")); + assert!(profile.contains("\"prefix+9\"")); + assert!(!switch_tab_labels.contains(&"prefix+1")); + assert_eq!(switch_tab_labels.len(), 8); + assert!(switch_tab_labels + .iter() + .all(|label| label.starts_with("prefix+"))); + } + #[test] fn remote_image_paste_key_defaults_to_ctrl_v() { let config = Config::default(); diff --git a/src/config/keybinds.rs b/src/config/keybinds.rs index c3e15a8b..2a1cfc49 100644 --- a/src/config/keybinds.rs +++ b/src/config/keybinds.rs @@ -43,6 +43,35 @@ impl BindingConfig { Self::Many(values) => values.iter().map(String::as_str).collect(), } } + + pub(crate) fn has_values(&self) -> bool { + self.values().iter().any(|value| !value.trim().is_empty()) + } + + pub(crate) fn indexed_labels(&self) -> Vec { + let mut labels = Vec::new(); + for raw in self.values() { + let raw = raw.trim(); + if raw.is_empty() { + continue; + } + match parse_binding_string(raw) { + Some(ParsedBinding::Single(binding)) => { + if matches!(binding.trigger.combo().0, KeyCode::Char('1'..='9')) { + labels.push(binding.label); + } + } + Some(ParsedBinding::Range(range)) => { + labels.extend(range.into_iter().filter_map(|binding| { + matches!(binding.trigger.combo().0, KeyCode::Char('1'..='9')) + .then_some(binding.label) + })); + } + None => {} + } + } + labels + } } #[derive(Debug, Clone, Copy, PartialEq, Eq, Deserialize, Serialize, Default)] @@ -321,53 +350,65 @@ enum ParsedBinding { Range(Vec), } +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +enum BindingSource { + Default, + User, +} + +struct RegisteredBinding { + field: String, + source: BindingSource, +} + struct BindingRegistry { prefix_combo: KeyCombo, - direct: std::collections::HashMap, - prefix: std::collections::HashMap, + prefix_source: BindingSource, + direct: std::collections::HashMap, + prefix: std::collections::HashMap, } impl BindingRegistry { - fn new(prefix_combo: KeyCombo) -> Self { + fn new(prefix_combo: KeyCombo, prefix_source: BindingSource) -> Self { Self { prefix_combo: normalize_key_combo(prefix_combo), + prefix_source, direct: std::collections::HashMap::new(), prefix: std::collections::HashMap::new(), } } - fn reserve_direct(&mut self, combo: KeyCombo, field: &str) { + fn reserve_direct(&mut self, combo: KeyCombo, field: &str, source: BindingSource) { self.direct .entry(normalize_key_combo(combo)) - .or_insert_with(|| field.to_string()); + .or_insert_with(|| RegisteredBinding { + field: field.to_string(), + source, + }); } fn prefix_rhs_is_reserved(&self, combo: KeyCombo) -> bool { normalize_key_combo(combo) == self.prefix_combo } - fn conflict(&self, binding: &ResolvedBinding) -> Option<&str> { + fn conflict(&self, binding: &ResolvedBinding) -> Option<&RegisteredBinding> { match binding.trigger { - BindingTrigger::Direct(combo) => self - .direct - .get(&normalize_key_combo(combo)) - .map(String::as_str), - BindingTrigger::Prefix(combo) => self - .prefix - .get(&normalize_key_combo(combo)) - .map(String::as_str), + BindingTrigger::Direct(combo) => self.direct.get(&normalize_key_combo(combo)), + BindingTrigger::Prefix(combo) => self.prefix.get(&normalize_key_combo(combo)), } } - fn register(&mut self, binding: &ResolvedBinding, field: &str) { + fn register(&mut self, binding: &ResolvedBinding, field: &str, source: BindingSource) { + let registered = || RegisteredBinding { + field: field.to_string(), + source, + }; match binding.trigger { BindingTrigger::Direct(combo) => { - self.direct - .insert(normalize_key_combo(combo), field.to_string()); + self.direct.insert(normalize_key_combo(combo), registered()); } BindingTrigger::Prefix(combo) => { - self.prefix - .insert(normalize_key_combo(combo), field.to_string()); + self.prefix.insert(normalize_key_combo(combo), registered()); } } } @@ -385,174 +426,254 @@ impl Config { warn!(message = %diag, "config diagnostic"); } - let mut registry = BindingRegistry::new(prefix); - registry.reserve_direct(prefix, "keys.prefix"); - let mut navigate_registry = BindingRegistry::new(prefix); - navigate_registry.reserve_direct(prefix, "keys.prefix"); + let prefix_source = if self.keys.key_field_is_user_configured("prefix") { + BindingSource::User + } else { + BindingSource::Default + }; + let mut registry = BindingRegistry::new(prefix, prefix_source); + registry.reserve_direct(prefix, "keys.prefix", prefix_source); + let mut navigate_registry = BindingRegistry::new(prefix, prefix_source); + navigate_registry.reserve_direct(prefix, "keys.prefix", prefix_source); reserve_navigate_runtime_keys(&mut navigate_registry); - macro_rules! action { - ($field:literal, $config:expr) => { - parse_action_bindings($field, $config, false, &mut registry, &mut diagnostics) - }; - } - macro_rules! indexed { - ($field:literal, $config:expr) => { - parse_indexed_bindings($field, $config, &mut registry, &mut diagnostics) + macro_rules! empty_action { + () => { + ActionKeybinds::default() }; } let mut keybinds = Keybinds { navigate: NavigateKeybinds { - workspace_up: parse_navigate_bindings( - "keys.navigate_workspace_up", - &self.keys.navigate_workspace_up, - &mut navigate_registry, - &mut diagnostics, - ), - workspace_down: parse_navigate_bindings( - "keys.navigate_workspace_down", - &self.keys.navigate_workspace_down, - &mut navigate_registry, - &mut diagnostics, - ), - pane_left: parse_navigate_bindings( - "keys.navigate_pane_left", - &self.keys.navigate_pane_left, - &mut navigate_registry, - &mut diagnostics, - ), - pane_down: parse_navigate_bindings( - "keys.navigate_pane_down", - &self.keys.navigate_pane_down, - &mut navigate_registry, - &mut diagnostics, - ), - pane_up: parse_navigate_bindings( - "keys.navigate_pane_up", - &self.keys.navigate_pane_up, - &mut navigate_registry, - &mut diagnostics, - ), - pane_right: parse_navigate_bindings( - "keys.navigate_pane_right", - &self.keys.navigate_pane_right, - &mut navigate_registry, - &mut diagnostics, - ), + workspace_up: empty_action!(), + workspace_down: empty_action!(), + pane_left: empty_action!(), + pane_down: empty_action!(), + pane_up: empty_action!(), + pane_right: empty_action!(), }, - help: action!("keys.help", &self.keys.help), - settings: action!("keys.settings", &self.keys.settings), - new_workspace: action!("keys.new_workspace", &self.keys.new_workspace), - new_worktree: action!("keys.new_worktree", &self.keys.new_worktree), - open_worktree: action!("keys.open_worktree", &self.keys.open_worktree), - remove_worktree: action!("keys.remove_worktree", &self.keys.remove_worktree), - rename_workspace: action!("keys.rename_workspace", &self.keys.rename_workspace), - close_workspace: action!("keys.close_workspace", &self.keys.close_workspace), - workspace_picker: action!("keys.workspace_picker", &self.keys.workspace_picker), - goto: action!("keys.goto", &self.keys.goto), - detach: action!("keys.detach", &self.keys.detach), - reload_config: action!("keys.reload_config", &self.keys.reload_config), - open_notification_target: action!( - "keys.open_notification_target", - &self.keys.open_notification_target - ), - previous_workspace: action!("keys.previous_workspace", &self.keys.previous_workspace), - next_workspace: action!("keys.next_workspace", &self.keys.next_workspace), - previous_agent: action!("keys.previous_agent", &self.keys.previous_agent), - next_agent: action!("keys.next_agent", &self.keys.next_agent), - focus_agent: indexed!("keys.focus_agent", &self.keys.focus_agent), - new_tab: action!("keys.new_tab", &self.keys.new_tab), - rename_tab: action!("keys.rename_tab", &self.keys.rename_tab), - previous_tab: action!("keys.previous_tab", &self.keys.previous_tab), - next_tab: action!("keys.next_tab", &self.keys.next_tab), - switch_tab: indexed!("keys.switch_tab", &self.keys.switch_tab), - switch_workspace: indexed!("keys.switch_workspace", &self.keys.switch_workspace), - close_tab: action!("keys.close_tab", &self.keys.close_tab), - rename_pane: action!("keys.rename_pane", &self.keys.rename_pane), - edit_scrollback: action!("keys.edit_scrollback", &self.keys.edit_scrollback), - copy_mode: action!("keys.copy_mode", &self.keys.copy_mode), - focus_pane_left: action!("keys.focus_pane_left", &self.keys.focus_pane_left), - focus_pane_down: action!("keys.focus_pane_down", &self.keys.focus_pane_down), - focus_pane_up: action!("keys.focus_pane_up", &self.keys.focus_pane_up), - focus_pane_right: action!("keys.focus_pane_right", &self.keys.focus_pane_right), - swap_pane_left: action!("keys.swap_pane_left", &self.keys.swap_pane_left), - swap_pane_down: action!("keys.swap_pane_down", &self.keys.swap_pane_down), - swap_pane_up: action!("keys.swap_pane_up", &self.keys.swap_pane_up), - swap_pane_right: action!("keys.swap_pane_right", &self.keys.swap_pane_right), - last_pane: action!("keys.last_pane", &self.keys.last_pane), - cycle_pane_next: action!("keys.cycle_pane_next", &self.keys.cycle_pane_next), - cycle_pane_previous: action!( - "keys.cycle_pane_previous", - &self.keys.cycle_pane_previous - ), - split_vertical: action!("keys.split_vertical", &self.keys.split_vertical), - split_horizontal: action!("keys.split_horizontal", &self.keys.split_horizontal), - close_pane: action!("keys.close_pane", &self.keys.close_pane), - zoom: action!("keys.zoom", &self.keys.zoom), - resize_mode: action!("keys.resize_mode", &self.keys.resize_mode), - toggle_sidebar: action!("keys.toggle_sidebar", &self.keys.toggle_sidebar), + help: empty_action!(), + settings: empty_action!(), + new_workspace: empty_action!(), + new_worktree: empty_action!(), + open_worktree: empty_action!(), + remove_worktree: empty_action!(), + rename_workspace: empty_action!(), + close_workspace: empty_action!(), + workspace_picker: empty_action!(), + goto: empty_action!(), + detach: empty_action!(), + reload_config: empty_action!(), + open_notification_target: empty_action!(), + previous_workspace: empty_action!(), + next_workspace: empty_action!(), + previous_agent: empty_action!(), + next_agent: empty_action!(), + focus_agent: Vec::new(), + new_tab: empty_action!(), + rename_tab: empty_action!(), + previous_tab: empty_action!(), + next_tab: empty_action!(), + switch_tab: Vec::new(), + switch_workspace: Vec::new(), + close_tab: empty_action!(), + rename_pane: empty_action!(), + edit_scrollback: empty_action!(), + copy_mode: empty_action!(), + focus_pane_left: empty_action!(), + focus_pane_down: empty_action!(), + focus_pane_up: empty_action!(), + focus_pane_right: empty_action!(), + swap_pane_left: empty_action!(), + swap_pane_down: empty_action!(), + swap_pane_up: empty_action!(), + swap_pane_right: empty_action!(), + cycle_pane_next: empty_action!(), + cycle_pane_previous: empty_action!(), + last_pane: empty_action!(), + split_vertical: empty_action!(), + split_horizontal: empty_action!(), + close_pane: empty_action!(), + zoom: empty_action!(), + resize_mode: empty_action!(), + toggle_sidebar: empty_action!(), custom_commands: Vec::new(), }; - append_legacy_indexed_bindings( - &mut keybinds.switch_tab, - "keys.indexed.tabs", - &self.keys.indexed.tabs, - &mut registry, - &mut diagnostics, - ); - append_legacy_indexed_bindings( - &mut keybinds.switch_workspace, - "keys.indexed.workspaces", - &self.keys.indexed.workspaces, - &mut registry, - &mut diagnostics, - ); - append_legacy_indexed_bindings( - &mut keybinds.focus_agent, - "keys.indexed.agents", - &self.keys.indexed.agents, - &mut registry, - &mut diagnostics, - ); - - for (index, command) in self.keys.command.iter().enumerate() { - let key_field = format!("keys.command[{index}].key"); - let command_field = format!("keys.command[{index}].command"); - - if command.command.trim().is_empty() { - let diag = - format!("empty custom command: {command_field}; disabling custom command"); - warn!(message = %diag, "config diagnostic"); - diagnostics.push(diag); - continue; - } - - let bindings = parse_action_bindings_owned( - &key_field, - &command.key, - false, - &mut registry, - &mut diagnostics, - ); - if bindings.bindings.is_empty() { - continue; - } - - let action = match command.action_type { - CommandKeybindType::Shell => CustomCommandAction::Shell, - CommandKeybindType::Pane => CustomCommandAction::Pane, - CommandKeybindType::PluginAction => CustomCommandAction::PluginAction, + macro_rules! field_source { + ($field:ident) => { + if self.keys.key_field_is_user_configured(stringify!($field)) { + BindingSource::User + } else { + BindingSource::Default + } }; - let label = bindings.label().unwrap_or_else(|| "unset".to_string()); - keybinds.custom_commands.push(CustomCommandKeybind { - bindings, - label, - command: command.command.clone(), - action, - description: command.description.clone(), - }); + } + macro_rules! apply_action { + ($target:expr, $field:ident, $source:expr) => { + if field_source!($field) == $source { + $target = parse_action_bindings( + concat!("keys.", stringify!($field)), + &self.keys.$field, + &mut registry, + &mut diagnostics, + $source, + ); + } + }; + } + macro_rules! apply_indexed { + ( + $target:expr, + $field:ident, + $legacy_config:expr, + $source:expr + ) => { + if field_source!($field) == $source { + if $source == BindingSource::Default && !$legacy_config.trim().is_empty() { + // A legacy [keys.indexed] entry is user configuration for + // this target and should displace the modern default. + } else { + $target = parse_indexed_bindings( + concat!("keys.", stringify!($field)), + &self.keys.$field, + &mut registry, + &mut diagnostics, + $source, + ); + } + } + }; + } + macro_rules! apply_navigate { + ($target:expr, $field:ident, $source:expr) => { + if field_source!($field) == $source { + $target = parse_navigate_bindings( + concat!("keys.", stringify!($field)), + &self.keys.$field, + &mut navigate_registry, + &mut diagnostics, + $source, + ); + } + }; + } + + for source in [BindingSource::User, BindingSource::Default] { + apply_navigate!( + keybinds.navigate.workspace_up, + navigate_workspace_up, + source + ); + apply_navigate!( + keybinds.navigate.workspace_down, + navigate_workspace_down, + source + ); + apply_navigate!(keybinds.navigate.pane_left, navigate_pane_left, source); + apply_navigate!(keybinds.navigate.pane_down, navigate_pane_down, source); + apply_navigate!(keybinds.navigate.pane_up, navigate_pane_up, source); + apply_navigate!(keybinds.navigate.pane_right, navigate_pane_right, source); + apply_action!(keybinds.help, help, source); + apply_action!(keybinds.settings, settings, source); + apply_action!(keybinds.new_workspace, new_workspace, source); + apply_action!(keybinds.new_worktree, new_worktree, source); + apply_action!(keybinds.open_worktree, open_worktree, source); + apply_action!(keybinds.remove_worktree, remove_worktree, source); + apply_action!(keybinds.rename_workspace, rename_workspace, source); + apply_action!(keybinds.close_workspace, close_workspace, source); + apply_action!(keybinds.workspace_picker, workspace_picker, source); + apply_action!(keybinds.goto, goto, source); + apply_action!(keybinds.detach, detach, source); + apply_action!(keybinds.reload_config, reload_config, source); + apply_action!( + keybinds.open_notification_target, + open_notification_target, + source + ); + apply_action!(keybinds.previous_workspace, previous_workspace, source); + apply_action!(keybinds.next_workspace, next_workspace, source); + apply_action!(keybinds.previous_agent, previous_agent, source); + apply_action!(keybinds.next_agent, next_agent, source); + apply_indexed!( + keybinds.focus_agent, + focus_agent, + &self.keys.indexed.agents, + source + ); + apply_action!(keybinds.new_tab, new_tab, source); + apply_action!(keybinds.rename_tab, rename_tab, source); + apply_action!(keybinds.previous_tab, previous_tab, source); + apply_action!(keybinds.next_tab, next_tab, source); + apply_indexed!( + keybinds.switch_tab, + switch_tab, + &self.keys.indexed.tabs, + source + ); + apply_indexed!( + keybinds.switch_workspace, + switch_workspace, + &self.keys.indexed.workspaces, + source + ); + apply_action!(keybinds.close_tab, close_tab, source); + apply_action!(keybinds.rename_pane, rename_pane, source); + apply_action!(keybinds.edit_scrollback, edit_scrollback, source); + apply_action!(keybinds.copy_mode, copy_mode, source); + apply_action!(keybinds.focus_pane_left, focus_pane_left, source); + apply_action!(keybinds.focus_pane_down, focus_pane_down, source); + apply_action!(keybinds.focus_pane_up, focus_pane_up, source); + apply_action!(keybinds.focus_pane_right, focus_pane_right, source); + apply_action!(keybinds.swap_pane_left, swap_pane_left, source); + apply_action!(keybinds.swap_pane_down, swap_pane_down, source); + apply_action!(keybinds.swap_pane_up, swap_pane_up, source); + apply_action!(keybinds.swap_pane_right, swap_pane_right, source); + apply_action!(keybinds.last_pane, last_pane, source); + apply_action!(keybinds.cycle_pane_next, cycle_pane_next, source); + apply_action!(keybinds.cycle_pane_previous, cycle_pane_previous, source); + apply_action!(keybinds.split_vertical, split_vertical, source); + apply_action!(keybinds.split_horizontal, split_horizontal, source); + apply_action!(keybinds.close_pane, close_pane, source); + apply_action!(keybinds.zoom, zoom, source); + apply_action!(keybinds.resize_mode, resize_mode, source); + apply_action!(keybinds.toggle_sidebar, toggle_sidebar, source); + + if source == field_source!(indexed) { + append_legacy_indexed_bindings( + &mut keybinds.switch_tab, + "keys.indexed.tabs", + &self.keys.indexed.tabs, + &mut registry, + &mut diagnostics, + source, + ); + append_legacy_indexed_bindings( + &mut keybinds.switch_workspace, + "keys.indexed.workspaces", + &self.keys.indexed.workspaces, + &mut registry, + &mut diagnostics, + source, + ); + append_legacy_indexed_bindings( + &mut keybinds.focus_agent, + "keys.indexed.agents", + &self.keys.indexed.agents, + &mut registry, + &mut diagnostics, + source, + ); + } + + if source == BindingSource::User { + append_custom_command_bindings( + self, + &mut keybinds, + &mut registry, + &mut diagnostics, + ); + } } (prefix_diag, prefix, diagnostics, keybinds) @@ -569,33 +690,68 @@ fn reserve_navigate_runtime_keys(registry: &mut BindingRegistry) { (KeyCode::Left, KeyModifiers::empty()), (KeyCode::Right, KeyModifiers::empty()), ] { - registry.reserve_direct(combo, "navigate reserved keys"); + registry.reserve_direct(combo, "navigate reserved keys", BindingSource::Default); } for idx in '1'..='9' { registry.reserve_direct( (KeyCode::Char(idx), KeyModifiers::empty()), "navigate reserved keys", + BindingSource::Default, ); } } -fn parse_action_bindings( - field: &'static str, - config: &BindingConfig, - allow_ranges: bool, +fn append_custom_command_bindings( + config: &Config, + keybinds: &mut Keybinds, registry: &mut BindingRegistry, diagnostics: &mut Vec, -) -> ActionKeybinds { - parse_action_bindings_owned(field, config, allow_ranges, registry, diagnostics) +) { + for (index, command) in config.keys.command.iter().enumerate() { + let key_field = format!("keys.command[{index}].key"); + let command_field = format!("keys.command[{index}].command"); + + if command.command.trim().is_empty() { + let diag = format!("empty custom command: {command_field}; disabling custom command"); + warn!(message = %diag, "config diagnostic"); + diagnostics.push(diag); + continue; + } + + let bindings = parse_action_bindings( + &key_field, + &command.key, + registry, + diagnostics, + BindingSource::User, + ); + if bindings.bindings.is_empty() { + continue; + } + + let action = match command.action_type { + CommandKeybindType::Shell => CustomCommandAction::Shell, + CommandKeybindType::Pane => CustomCommandAction::Pane, + CommandKeybindType::PluginAction => CustomCommandAction::PluginAction, + }; + let label = bindings.label().unwrap_or_else(|| "unset".to_string()); + keybinds.custom_commands.push(CustomCommandKeybind { + bindings, + label, + command: command.command.clone(), + action, + description: command.description.clone(), + }); + } } -fn parse_action_bindings_owned( +fn parse_action_bindings( field: &str, config: &BindingConfig, - allow_ranges: bool, registry: &mut BindingRegistry, diagnostics: &mut Vec, + source: BindingSource, ) -> ActionKeybinds { let mut bindings = Vec::new(); for raw in config.values() { @@ -605,26 +761,17 @@ fn parse_action_bindings_owned( } match parse_binding_string(raw) { Some(ParsedBinding::Single(binding)) => { - if reject_binding(field, &binding, registry, diagnostics) { + if reject_binding(field, &binding, registry, diagnostics, source) { continue; } - registry.register(&binding, field); + registry.register(&binding, field, source); bindings.push(binding); } - Some(ParsedBinding::Range(_)) if !allow_ranges => { + Some(ParsedBinding::Range(_)) => { let diag = format!("range keybinding is only valid for indexed actions: {field} = {raw:?}; disabling binding"); warn!(message = %diag, "config diagnostic"); diagnostics.push(diag); } - Some(ParsedBinding::Range(range)) => { - for binding in range { - if reject_binding(field, &binding, registry, diagnostics) { - continue; - } - registry.register(&binding, field); - bindings.push(binding); - } - } None => { let diag = format!("invalid keybinding: {field} = {raw:?}; disabling binding"); warn!(message = %diag, "config diagnostic"); @@ -640,6 +787,7 @@ fn parse_navigate_bindings( config: &BindingConfig, registry: &mut BindingRegistry, diagnostics: &mut Vec, + source: BindingSource, ) -> ActionKeybinds { let mut bindings = Vec::new(); for raw in config.values() { @@ -649,10 +797,10 @@ fn parse_navigate_bindings( } match parse_binding_string(raw) { Some(ParsedBinding::Single(binding)) => { - if reject_navigate_binding(field, &binding, registry, diagnostics) { + if reject_navigate_binding(field, &binding, registry, diagnostics, source) { continue; } - registry.register(&binding, field); + registry.register(&binding, field, source); bindings.push(binding); } Some(ParsedBinding::Range(_)) => { @@ -675,27 +823,65 @@ fn parse_indexed_bindings( config: &BindingConfig, registry: &mut BindingRegistry, diagnostics: &mut Vec, + source: BindingSource, ) -> Vec { - parse_action_bindings(field, config, true, registry, diagnostics) - .bindings - .into_iter() - .filter_map(|binding| { - if matches!(binding.trigger.combo().0, KeyCode::Char('1'..='9')) { - Some(IndexedKeybind { - trigger: binding.trigger, - label: binding.label, - }) - } else { - let diag = format!( - "indexed keybinding must use 1..9: {field} = {:?}; disabling binding", - binding.label - ); + let mut bindings = Vec::new(); + for raw in config.values() { + let raw = raw.trim(); + if raw.is_empty() { + continue; + } + match parse_binding_string(raw) { + Some(ParsedBinding::Single(binding)) => { + push_indexed_binding(field, binding, registry, diagnostics, source, &mut bindings); + } + Some(ParsedBinding::Range(range)) => { + for binding in range { + push_indexed_binding( + field, + binding, + registry, + diagnostics, + source, + &mut bindings, + ); + } + } + None => { + let diag = format!("invalid keybinding: {field} = {raw:?}; disabling binding"); warn!(message = %diag, "config diagnostic"); diagnostics.push(diag); - None } - }) - .collect() + } + } + bindings +} + +fn push_indexed_binding( + field: &str, + binding: ResolvedBinding, + registry: &mut BindingRegistry, + diagnostics: &mut Vec, + source: BindingSource, + bindings: &mut Vec, +) { + if !matches!(binding.trigger.combo().0, KeyCode::Char('1'..='9')) { + let diag = format!( + "indexed keybinding must use 1..9: {field} = {:?}; disabling binding", + binding.label + ); + warn!(message = %diag, "config diagnostic"); + diagnostics.push(diag); + return; + } + if reject_binding(field, &binding, registry, diagnostics, source) { + return; + } + registry.register(&binding, field, source); + bindings.push(IndexedKeybind { + trigger: binding.trigger, + label: binding.label, + }); } fn append_legacy_indexed_bindings( @@ -704,6 +890,7 @@ fn append_legacy_indexed_bindings( configured_label: &str, registry: &mut BindingRegistry, diagnostics: &mut Vec, + source: BindingSource, ) { if configured_label.trim().is_empty() { return; @@ -726,10 +913,10 @@ fn append_legacy_indexed_bindings( trigger: BindingTrigger::Direct(combo), label: format!("{}+{idx}", configured_label.trim()), }; - if reject_binding(field, &binding, registry, diagnostics) { + if reject_binding(field, &binding, registry, diagnostics, source) { continue; } - registry.register(&binding, field); + registry.register(&binding, field, source); target.push(IndexedKeybind { trigger: binding.trigger, label: binding.label, @@ -742,6 +929,7 @@ fn reject_navigate_binding( binding: &ResolvedBinding, registry: &BindingRegistry, diagnostics: &mut Vec, + source: BindingSource, ) -> bool { if binding.trigger.is_prefix() { let diag = format!( @@ -763,7 +951,11 @@ fn reject_navigate_binding( return true; } - if let Some(first_field) = registry.conflict(binding) { + if let Some(first_binding) = registry.conflict(binding) { + if source == BindingSource::Default && first_binding.source == BindingSource::User { + return true; + } + let first_field = &first_binding.field; let diag = format!("{}: kept {first_field}, disabled {field}", binding.label); warn!(message = %diag, "config diagnostic"); diagnostics.push(diag); @@ -778,8 +970,12 @@ fn reject_binding( binding: &ResolvedBinding, registry: &BindingRegistry, diagnostics: &mut Vec, + source: BindingSource, ) -> bool { if binding.trigger.is_prefix() && registry.prefix_rhs_is_reserved(binding.trigger.combo()) { + if source == BindingSource::Default && registry.prefix_source == BindingSource::User { + return true; + } let diag = format!( "reserved keybinding: {field} = {:?} uses keys.prefix as the prefix-mode key; pressing the prefix twice sends a literal prefix key, so this binding is disabled", binding.label @@ -789,7 +985,11 @@ fn reject_binding( return true; } - if let Some(first_field) = registry.conflict(binding) { + if let Some(first_binding) = registry.conflict(binding) { + if source == BindingSource::Default && first_binding.source == BindingSource::User { + return true; + } + let first_field = &first_binding.field; let diag = format!("{}: kept {first_field}, disabled {field}", binding.label); warn!(message = %diag, "config diagnostic"); diagnostics.push(diag); @@ -1718,6 +1918,76 @@ switch_workspace = "prefix+shift+1..9" assert_eq!(kb.switch_workspace[0].label, "prefix+shift+1"); } + #[test] + fn legacy_indexed_user_bindings_displace_modern_defaults() { + let config: Config = toml::from_str( + r#" +[keys.indexed] +workspaces = "ctrl" +"#, + ) + .unwrap(); + + let diagnostics = config.collect_diagnostics(); + let kb = config.keybinds(); + + assert!(diagnostics.is_empty(), "{diagnostics:?}"); + assert_eq!(kb.switch_workspace.len(), 9); + assert_eq!( + kb.switch_workspace[0].trigger, + BindingTrigger::Direct((KeyCode::Char('1'), KeyModifiers::CONTROL)) + ); + assert_eq!(kb.switch_workspace[0].label, "ctrl+1"); + } + + #[test] + fn invalid_legacy_indexed_user_binding_displaces_modern_default() { + let config: Config = toml::from_str( + r#" +[keys.indexed] +tabs = "bogus" +"#, + ) + .unwrap(); + + let diagnostics = config.collect_diagnostics(); + let kb = config.keybinds(); + + assert!(kb.switch_tab.is_empty()); + assert!(diagnostics.iter().any(|diag| { + diag.contains("invalid indexed keybinding") && diag.contains("keys.indexed.tabs") + })); + } + + #[test] + fn invalid_indexed_binding_does_not_displace_default_binding() { + let config: Config = toml::from_str( + r#" +[keys] +switch_tab = "prefix+?" +"#, + ) + .unwrap(); + + let diagnostics = config.collect_diagnostics(); + let kb = config.keybinds(); + + assert!(kb.switch_tab.is_empty()); + assert_eq!( + binding_triggers(&kb.help), + vec![BindingTrigger::Prefix(( + KeyCode::Char('?'), + KeyModifiers::empty() + ))] + ); + assert!(diagnostics.iter().any(|diag| { + diag.contains("indexed keybinding must use 1..9") && diag.contains("keys.switch_tab") + })); + assert!(!diagnostics.iter().any(|diag| { + diag.contains("kept keys.switch_tab") && diag.contains("disabled keys.help") + })); + } + #[test] fn default_keymap_is_prefix_first_and_tab_centered() { let kb = Config::default().keybinds(); @@ -1793,6 +2063,75 @@ new_workspace = "prefix+n" })); } + #[test] + fn user_binding_silently_displaces_default_binding() { + let config: Config = toml::from_str( + r#" +[keys] +previous_workspace = "prefix+shift+l" +"#, + ) + .unwrap(); + + let diagnostics = config.collect_diagnostics(); + let kb = config.keybinds(); + + assert!(diagnostics.is_empty(), "{diagnostics:?}"); + assert_eq!( + binding_triggers(&kb.previous_workspace), + vec![BindingTrigger::Prefix(( + KeyCode::Char('l'), + KeyModifiers::SHIFT + ))] + ); + assert!(kb.swap_pane_right.bindings.is_empty()); + } + + #[test] + fn user_prefix_silently_displaces_default_prefix_rhs_binding() { + let config: Config = toml::from_str( + r#" +[keys] +prefix = "n" +"#, + ) + .unwrap(); + + let diagnostics = config.collect_diagnostics(); + let kb = config.keybinds(); + + assert!(diagnostics.is_empty(), "{diagnostics:?}"); + assert!(kb.next_tab.bindings.is_empty()); + } + + #[test] + fn duplicate_user_binding_still_reports_conflict() { + let config: Config = toml::from_str( + r#" +[keys] +previous_workspace = "prefix+shift+l" +swap_pane_right = "prefix+shift+l" +"#, + ) + .unwrap(); + + let diagnostics = config.collect_diagnostics(); + let kb = config.keybinds(); + + assert_eq!( + binding_triggers(&kb.previous_workspace), + vec![BindingTrigger::Prefix(( + KeyCode::Char('l'), + KeyModifiers::SHIFT + ))] + ); + assert!(kb.swap_pane_right.bindings.is_empty()); + assert!(diagnostics.iter().any(|diag| { + diag.contains("kept keys.previous_workspace") + && diag.contains("disabled keys.swap_pane_right") + })); + } + #[test] fn custom_command_with_description_parses() { let config: Config = toml::from_str( diff --git a/src/config/model.rs b/src/config/model.rs index 5396e26b..51af278f 100644 --- a/src/config/model.rs +++ b/src/config/model.rs @@ -1,11 +1,12 @@ -use std::num::NonZeroUsize; +use std::{collections::BTreeSet, num::NonZeroUsize}; use crossterm::event::KeyModifiers; use serde::{de, Deserialize, Deserializer, Serialize}; use super::{ - BindingConfig, CommandKeybindConfig, SoundConfig, ThemeConfig, DEFAULT_MOBILE_WIDTH_THRESHOLD, - DEFAULT_MOUSE_SCROLL_LINES, DEFAULT_SCROLLBACK_LIMIT_BYTES, + ActionKeybinds, BindingConfig, CommandKeybindConfig, IndexedKeybind, Keybinds, SoundConfig, + ThemeConfig, DEFAULT_MOBILE_WIDTH_THRESHOLD, DEFAULT_MOUSE_SCROLL_LINES, + DEFAULT_SCROLLBACK_LIMIT_BYTES, }; pub const MAX_TOAST_DELAY_SECONDS: u64 = 3600; @@ -287,8 +288,7 @@ pub struct LoadedConfig { pub invalid_sections: Vec, } -#[derive(Debug, Clone, Deserialize, Serialize)] -#[serde(default)] +#[derive(Debug, Clone, Serialize)] pub struct KeysConfig { /// Prefix key to enter prefix mode (e.g. "ctrl+b", "f12", "esc"). pub prefix: String, @@ -402,6 +402,337 @@ pub struct KeysConfig { /// Prefix-mode custom command bindings. #[serde(skip_serializing_if = "Vec::is_empty")] pub command: Vec, + #[serde(skip_serializing)] + pub(crate) user_fields: BTreeSet<&'static str>, +} + +#[derive(Debug, Default, Deserialize, Serialize)] +#[serde(default)] +pub(crate) struct KeysConfigOverlay { + #[serde(skip_serializing_if = "Option::is_none")] + prefix: Option, + #[serde(skip_serializing_if = "Option::is_none")] + help: Option, + #[serde(skip_serializing_if = "Option::is_none")] + settings: Option, + #[serde(skip_serializing_if = "Option::is_none")] + new_workspace: Option, + #[serde(skip_serializing_if = "Option::is_none")] + new_worktree: Option, + #[serde(skip_serializing_if = "Option::is_none")] + open_worktree: Option, + #[serde(skip_serializing_if = "Option::is_none")] + remove_worktree: Option, + #[serde(skip_serializing_if = "Option::is_none")] + rename_workspace: Option, + #[serde(skip_serializing_if = "Option::is_none")] + close_workspace: Option, + #[serde(skip_serializing_if = "Option::is_none")] + workspace_picker: Option, + #[serde(skip_serializing_if = "Option::is_none")] + goto: Option, + #[serde(skip_serializing_if = "Option::is_none")] + navigate_workspace_up: Option, + #[serde(skip_serializing_if = "Option::is_none")] + navigate_workspace_down: Option, + #[serde(skip_serializing_if = "Option::is_none")] + navigate_pane_left: Option, + #[serde(skip_serializing_if = "Option::is_none")] + navigate_pane_down: Option, + #[serde(skip_serializing_if = "Option::is_none")] + navigate_pane_up: Option, + #[serde(skip_serializing_if = "Option::is_none")] + navigate_pane_right: Option, + #[serde(skip_serializing_if = "Option::is_none")] + detach: Option, + #[serde(skip_serializing_if = "Option::is_none")] + reload_config: Option, + #[serde(skip_serializing_if = "Option::is_none")] + open_notification_target: Option, + #[serde(skip_serializing_if = "Option::is_none")] + previous_workspace: Option, + #[serde(skip_serializing_if = "Option::is_none")] + next_workspace: Option, + #[serde(skip_serializing_if = "Option::is_none")] + previous_agent: Option, + #[serde(skip_serializing_if = "Option::is_none")] + next_agent: Option, + #[serde(skip_serializing_if = "Option::is_none")] + focus_agent: Option, + #[serde(skip_serializing_if = "Option::is_none")] + remote_image_paste: Option, + #[serde(skip_serializing_if = "Option::is_none")] + new_tab: Option, + #[serde(skip_serializing_if = "Option::is_none")] + rename_tab: Option, + #[serde(skip_serializing_if = "Option::is_none")] + previous_tab: Option, + #[serde(skip_serializing_if = "Option::is_none")] + next_tab: Option, + #[serde(skip_serializing_if = "Option::is_none")] + switch_tab: Option, + #[serde(skip_serializing_if = "Option::is_none")] + switch_workspace: Option, + #[serde(skip_serializing_if = "Option::is_none")] + close_tab: Option, + #[serde(skip_serializing_if = "Option::is_none")] + rename_pane: Option, + #[serde(skip_serializing_if = "Option::is_none")] + edit_scrollback: Option, + #[serde(skip_serializing_if = "Option::is_none")] + copy_mode: Option, + #[serde(skip_serializing_if = "Option::is_none")] + focus_pane_left: Option, + #[serde(skip_serializing_if = "Option::is_none")] + focus_pane_down: Option, + #[serde(skip_serializing_if = "Option::is_none")] + focus_pane_up: Option, + #[serde(skip_serializing_if = "Option::is_none")] + focus_pane_right: Option, + #[serde(skip_serializing_if = "Option::is_none")] + swap_pane_left: Option, + #[serde(skip_serializing_if = "Option::is_none")] + swap_pane_down: Option, + #[serde(skip_serializing_if = "Option::is_none")] + swap_pane_up: Option, + #[serde(skip_serializing_if = "Option::is_none")] + swap_pane_right: Option, + #[serde(skip_serializing_if = "Option::is_none")] + cycle_pane_next: Option, + #[serde(skip_serializing_if = "Option::is_none")] + cycle_pane_previous: Option, + #[serde(skip_serializing_if = "Option::is_none")] + last_pane: Option, + #[serde(skip_serializing_if = "Option::is_none")] + split_vertical: Option, + #[serde(skip_serializing_if = "Option::is_none")] + split_horizontal: Option, + #[serde(skip_serializing_if = "Option::is_none")] + close_pane: Option, + #[serde(alias = "fullscreen", skip_serializing_if = "Option::is_none")] + zoom: Option, + #[serde(skip_serializing_if = "Option::is_none")] + resize_mode: Option, + #[serde(skip_serializing_if = "Option::is_none")] + toggle_sidebar: Option, + #[serde(skip_serializing_if = "Option::is_none")] + indexed: Option, + #[serde(skip_serializing)] + command: Option>, +} + +impl<'de> Deserialize<'de> for KeysConfig { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + let input = KeysConfigOverlay::deserialize(deserializer)?; + let mut keys = KeysConfig::default(); + + macro_rules! apply_field { + ($field:ident) => { + if let Some(value) = input.$field { + keys.$field = value; + keys.user_fields.insert(stringify!($field)); + } + }; + } + + apply_field!(prefix); + apply_field!(help); + apply_field!(settings); + apply_field!(new_workspace); + apply_field!(new_worktree); + apply_field!(open_worktree); + apply_field!(remove_worktree); + apply_field!(rename_workspace); + apply_field!(close_workspace); + apply_field!(workspace_picker); + apply_field!(goto); + apply_field!(navigate_workspace_up); + apply_field!(navigate_workspace_down); + apply_field!(navigate_pane_left); + apply_field!(navigate_pane_down); + apply_field!(navigate_pane_up); + apply_field!(navigate_pane_right); + apply_field!(detach); + apply_field!(reload_config); + apply_field!(open_notification_target); + apply_field!(previous_workspace); + apply_field!(next_workspace); + apply_field!(previous_agent); + apply_field!(next_agent); + apply_field!(focus_agent); + apply_field!(remote_image_paste); + apply_field!(new_tab); + apply_field!(rename_tab); + apply_field!(previous_tab); + apply_field!(next_tab); + apply_field!(switch_tab); + apply_field!(switch_workspace); + apply_field!(close_tab); + apply_field!(rename_pane); + apply_field!(edit_scrollback); + apply_field!(copy_mode); + apply_field!(focus_pane_left); + apply_field!(focus_pane_down); + apply_field!(focus_pane_up); + apply_field!(focus_pane_right); + apply_field!(swap_pane_left); + apply_field!(swap_pane_down); + apply_field!(swap_pane_up); + apply_field!(swap_pane_right); + apply_field!(cycle_pane_next); + apply_field!(cycle_pane_previous); + apply_field!(last_pane); + apply_field!(split_vertical); + apply_field!(split_horizontal); + apply_field!(close_pane); + apply_field!(zoom); + apply_field!(resize_mode); + apply_field!(toggle_sidebar); + apply_field!(indexed); + apply_field!(command); + + Ok(keys) + } +} + +impl KeysConfig { + pub(crate) fn key_field_is_user_configured(&self, field: &str) -> bool { + self.user_fields.contains(field) + } + + pub(crate) fn local_profile(&self, keybinds: &Keybinds) -> KeysConfigOverlay { + let mut profile = KeysConfigOverlay::default(); + + macro_rules! copy_user_field { + ($field:ident) => { + if self.user_fields.contains(stringify!($field)) { + profile.$field = Some(self.$field.clone()); + } + }; + } + macro_rules! copy_effective_action_field { + ($field:ident, $target:expr) => { + if self.user_fields.contains(stringify!($field)) { + profile.$field = Some(self.$field.clone()); + } else if binding_config_is_effective(&self.$field, &$target) { + profile.$field = Some(self.$field.clone()); + } else if binding_config_has_values(&self.$field) { + profile.$field = Some(BindingConfig::empty()); + } + }; + } + macro_rules! copy_effective_indexed_field { + ($field:ident, $target:expr) => { + if self.user_fields.contains(stringify!($field)) { + profile.$field = Some(self.$field.clone()); + } else if let Some(effective) = effective_indexed_config(&self.$field, &$target) { + profile.$field = Some(effective); + } else if binding_config_has_values(&self.$field) { + profile.$field = Some(BindingConfig::empty()); + } + }; + } + + profile.prefix = Some(self.prefix.clone()); + copy_effective_action_field!(help, keybinds.help); + copy_effective_action_field!(settings, keybinds.settings); + copy_effective_action_field!(new_workspace, keybinds.new_workspace); + copy_effective_action_field!(new_worktree, keybinds.new_worktree); + copy_effective_action_field!(open_worktree, keybinds.open_worktree); + copy_effective_action_field!(remove_worktree, keybinds.remove_worktree); + copy_effective_action_field!(rename_workspace, keybinds.rename_workspace); + copy_effective_action_field!(close_workspace, keybinds.close_workspace); + copy_effective_action_field!(workspace_picker, keybinds.workspace_picker); + copy_effective_action_field!(goto, keybinds.goto); + copy_effective_action_field!(navigate_workspace_up, keybinds.navigate.workspace_up); + copy_effective_action_field!(navigate_workspace_down, keybinds.navigate.workspace_down); + copy_effective_action_field!(navigate_pane_left, keybinds.navigate.pane_left); + copy_effective_action_field!(navigate_pane_down, keybinds.navigate.pane_down); + copy_effective_action_field!(navigate_pane_up, keybinds.navigate.pane_up); + copy_effective_action_field!(navigate_pane_right, keybinds.navigate.pane_right); + copy_effective_action_field!(detach, keybinds.detach); + copy_effective_action_field!(reload_config, keybinds.reload_config); + copy_effective_action_field!(open_notification_target, keybinds.open_notification_target); + copy_effective_action_field!(previous_workspace, keybinds.previous_workspace); + copy_effective_action_field!(next_workspace, keybinds.next_workspace); + copy_effective_action_field!(previous_agent, keybinds.previous_agent); + copy_effective_action_field!(next_agent, keybinds.next_agent); + copy_effective_indexed_field!(focus_agent, keybinds.focus_agent); + copy_user_field!(remote_image_paste); + copy_effective_action_field!(new_tab, keybinds.new_tab); + copy_effective_action_field!(rename_tab, keybinds.rename_tab); + copy_effective_action_field!(previous_tab, keybinds.previous_tab); + copy_effective_action_field!(next_tab, keybinds.next_tab); + copy_effective_indexed_field!(switch_tab, keybinds.switch_tab); + copy_effective_indexed_field!(switch_workspace, keybinds.switch_workspace); + copy_effective_action_field!(close_tab, keybinds.close_tab); + copy_effective_action_field!(rename_pane, keybinds.rename_pane); + copy_effective_action_field!(edit_scrollback, keybinds.edit_scrollback); + copy_effective_action_field!(copy_mode, keybinds.copy_mode); + copy_effective_action_field!(focus_pane_left, keybinds.focus_pane_left); + copy_effective_action_field!(focus_pane_down, keybinds.focus_pane_down); + copy_effective_action_field!(focus_pane_up, keybinds.focus_pane_up); + copy_effective_action_field!(focus_pane_right, keybinds.focus_pane_right); + copy_effective_action_field!(swap_pane_left, keybinds.swap_pane_left); + copy_effective_action_field!(swap_pane_down, keybinds.swap_pane_down); + copy_effective_action_field!(swap_pane_up, keybinds.swap_pane_up); + copy_effective_action_field!(swap_pane_right, keybinds.swap_pane_right); + copy_effective_action_field!(cycle_pane_next, keybinds.cycle_pane_next); + copy_effective_action_field!(cycle_pane_previous, keybinds.cycle_pane_previous); + copy_effective_action_field!(last_pane, keybinds.last_pane); + copy_effective_action_field!(split_vertical, keybinds.split_vertical); + copy_effective_action_field!(split_horizontal, keybinds.split_horizontal); + copy_effective_action_field!(close_pane, keybinds.close_pane); + copy_effective_action_field!(zoom, keybinds.zoom); + copy_effective_action_field!(resize_mode, keybinds.resize_mode); + copy_effective_action_field!(toggle_sidebar, keybinds.toggle_sidebar); + copy_user_field!(indexed); + + profile + } +} + +fn binding_config_has_values(config: &BindingConfig) -> bool { + config.has_values() +} + +fn binding_config_is_effective(config: &BindingConfig, keybinds: &ActionKeybinds) -> bool { + !binding_config_has_values(config) || !keybinds.bindings.is_empty() +} + +fn effective_indexed_config( + config: &BindingConfig, + keybinds: &[IndexedKeybind], +) -> Option { + if !binding_config_has_values(config) { + return Some(config.clone()); + } + + let expected_labels = config.indexed_labels(); + if expected_labels.is_empty() { + return None; + } + + let effective_labels: Vec = expected_labels + .iter() + .filter(|expected| { + keybinds + .iter() + .any(|binding| binding.label.as_str() == expected.as_str()) + }) + .cloned() + .collect(); + + if effective_labels.is_empty() { + None + } else if effective_labels.len() == expected_labels.len() { + Some(config.clone()) + } else { + Some(BindingConfig::Many(effective_labels)) + } } #[derive(Debug, Default, Clone, Deserialize, Serialize)] @@ -611,6 +942,7 @@ impl Default for KeysConfig { toggle_sidebar: BindingConfig::one("prefix+b"), indexed: IndexedKeysConfig::default(), command: Vec::new(), + user_fields: BTreeSet::new(), } } } diff --git a/src/server/headless.rs b/src/server/headless.rs index 0daa2444..36519d47 100644 --- a/src/server/headless.rs +++ b/src/server/headless.rs @@ -4272,7 +4272,8 @@ next_tab = "" } #[test] - fn invalid_server_keybindings_do_not_cache_local_keybindings_after_settings_save() { + fn invalid_server_keybindings_apply_valid_subset_after_settings_save_without_caching_local_keybindings( + ) { let path = std::env::temp_dir().join(format!( "herdr-headless-invalid-settings-{}-{}.toml", std::process::id(), @@ -4338,16 +4339,17 @@ next_tab = "" })); assert_eq!( server.app.state.prefix_code, - crossterm::event::KeyCode::Char('c') + crossterm::event::KeyCode::Char('b') ); - assert!(server + assert!(!server .app .state .keybinds .new_workspace .bindings .iter() - .any(|binding| binding.label == "prefix+m")); + .any(|binding| binding.label == "prefix+n")); + assert!(server.app.state.keybinds.new_workspace.bindings.is_empty()); std::env::remove_var(crate::config::CONFIG_PATH_ENV_VAR); let _ = std::fs::remove_file(path);