refactor(config)!: simplify keybindings configuration syntax (#404)

BREAKING CHANGE: mode keybindings dropped in favor of a global table

**What this means in practice:**
```toml
[keybindings.Channel]
quit = ["esc", "ctrl-c"]
# ...

[keybindings.RemoteControl]
quit = ["esc", "ctrl-c"]
# ...

[keybindings.SendToChannel]
quit = ["esc", "ctrl-c"]
# ...
```
are being replaced with
```toml
[keybindings]
quit = ["esc", "ctrl-c"]
# ...
```

Mode keybindings were I believe a premature optimization which only
brought additional complexity and redundancy to the code and did not
provide any real functionality in the current state of things for end
users.
This commit is contained in:
Alexandre Pasmantier 2025-03-19 01:35:29 +01:00 committed by GitHub
parent 47ea5a2b68
commit 7a85728da6
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 235 additions and 203 deletions

View File

@ -72,7 +72,7 @@ theme = "TwoDark"
# #
# Channel mode # Channel mode
# ------------------------ # ------------------------
[keybindings.Channel] [keybindings]
# Quit the application # Quit the application
quit = ["esc", "ctrl-c"] quit = ["esc", "ctrl-c"]
# Scrolling through entries # Scrolling through entries
@ -101,46 +101,6 @@ toggle_help = "ctrl-g"
toggle_preview = "ctrl-o" toggle_preview = "ctrl-o"
# Remote control mode
# -------------------------------
[keybindings.RemoteControl]
# Quit the application
quit = "esc"
# Scrolling through entries
select_next_entry = ["down", "ctrl-n", "ctrl-j"]
select_prev_entry = ["up", "ctrl-p", "ctrl-k"]
select_next_page = "pagedown"
select_prev_page = "pageup"
# Select an entry
select_entry = "enter"
# Toggle the remote control mode
toggle_remote_control = "ctrl-r"
# Toggle the help bar
toggle_help = "ctrl-g"
# Toggle the preview panel
toggle_preview = "ctrl-o"
# Send to channel mode
# --------------------------------
[keybindings.SendToChannel]
# Quit the application
quit = "esc"
# Scrolling through entries
select_next_entry = ["down", "ctrl-n", "ctrl-j"]
select_prev_entry = ["up", "ctrl-p", "ctrl-k"]
select_next_page = "pagedown"
select_prev_page = "pageup"
# Select an entry
select_entry = "enter"
# Toggle the send to channel mode
toggle_send_to_channel = "ctrl-s"
# Toggle the help bar
toggle_help = "ctrl-g"
# Toggle the preview panel
toggle_preview = "ctrl-o"
# Shell integration # Shell integration
# ---------------------------------------------------------------------------- # ----------------------------------------------------------------------------
# #

View File

@ -6,7 +6,9 @@ use tracing::{debug, info, trace};
use crate::channels::entry::Entry; use crate::channels::entry::Entry;
use crate::channels::TelevisionChannel; use crate::channels::TelevisionChannel;
use crate::config::{parse_key, Config}; use crate::config::{
merge_keybindings, parse_key, Binding, Config, KeyBindings,
};
use crate::keymap::Keymap; use crate::keymap::Keymap;
use crate::render::UiState; use crate::render::UiState;
use crate::television::{Mode, Television}; use crate::television::{Mode, Television};
@ -109,16 +111,22 @@ impl App {
let (_, event_rx) = mpsc::unbounded_channel(); let (_, event_rx) = mpsc::unbounded_channel();
let (event_abort_tx, _) = mpsc::unbounded_channel(); let (event_abort_tx, _) = mpsc::unbounded_channel();
let tick_rate = config.config.tick_rate; let tick_rate = config.config.tick_rate;
let keymap = Keymap::from(&config.keybindings).with_mode_mappings( let keybindings = merge_keybindings(config.keybindings.clone(), {
Mode::Channel, &KeyBindings::from(passthrough_keybindings.iter().filter_map(
passthrough_keybindings |s| match parse_key(s) {
.iter() Ok(key) => Some((
.flat_map(|s| match parse_key(s) { Action::SelectPassthrough(s.to_string()),
Ok(key) => Ok((key, Action::SelectPassthrough(s.clone()))), Binding::SingleKey(key),
Err(e) => Err(e), )),
}) Err(e) => {
.collect(), debug!("Failed to parse keybinding: {}", e);
); None
}
},
))
});
let keymap = Keymap::from(&keybindings);
debug!("{:?}", keymap); debug!("{:?}", keymap);
let (ui_state_tx, ui_state_rx) = mpsc::unbounded_channel(); let (ui_state_tx, ui_state_rx) = mpsc::unbounded_channel();
let television = let television =
@ -258,14 +266,13 @@ impl App {
_ => {} _ => {}
} }
// get action based on keybindings // get action based on keybindings
self.keymap self.keymap.get(&keycode).cloned().unwrap_or(
.get(&self.television.mode) if let Key::Char(c) = keycode {
.and_then(|keymap| keymap.get(&keycode).cloned())
.unwrap_or(if let Key::Char(c) = keycode {
Action::AddInputChar(c) Action::AddInputChar(c)
} else { } else {
Action::NoOp Action::NoOp
}) },
)
} }
// terminal events // terminal events
Event::Tick => Action::Tick, Event::Tick => Action::Tick,

View File

@ -1,6 +1,5 @@
use crate::action::Action; use crate::action::Action;
use crate::event::{convert_raw_event_to_key, Key}; use crate::event::{convert_raw_event_to_key, Key};
use crate::television::Mode;
use crossterm::event::{KeyCode, KeyEvent, KeyModifiers}; use crossterm::event::{KeyCode, KeyEvent, KeyModifiers};
use rustc_hash::FxHashMap; use rustc_hash::FxHashMap;
use serde::{Deserialize, Deserializer}; use serde::{Deserialize, Deserializer};
@ -30,7 +29,16 @@ impl Display for Binding {
} }
#[derive(Clone, Debug, Default, PartialEq)] #[derive(Clone, Debug, Default, PartialEq)]
pub struct KeyBindings(pub FxHashMap<Mode, FxHashMap<Action, Binding>>); pub struct KeyBindings(pub FxHashMap<Action, Binding>);
impl<I> From<I> for KeyBindings
where
I: IntoIterator<Item = (Action, Binding)>,
{
fn from(iter: I) -> Self {
KeyBindings(iter.into_iter().collect())
}
}
impl Hash for KeyBindings { impl Hash for KeyBindings {
fn hash<H: std::hash::Hasher>(&self, state: &mut H) { fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
@ -40,7 +48,7 @@ impl Hash for KeyBindings {
} }
impl Deref for KeyBindings { impl Deref for KeyBindings {
type Target = FxHashMap<Mode, FxHashMap<Action, Binding>>; type Target = FxHashMap<Action, Binding>;
fn deref(&self) -> &Self::Target { fn deref(&self) -> &Self::Target {
&self.0 &self.0
} }
@ -52,27 +60,20 @@ impl DerefMut for KeyBindings {
} }
} }
/// Merge two sets of keybindings together.
///
/// Note that this function won't "meld", for a given action, the bindings from the first set
/// with the bindings from the second set. Instead, it will simply overwrite them with the second
/// set's keys.
/// This is because it is assumed that the second set will be the user's custom keybindings, and
/// they should take precedence over the default ones, effectively replacing them to avoid
/// conflicts.
pub fn merge_keybindings( pub fn merge_keybindings(
mut keybindings: KeyBindings, mut keybindings: KeyBindings,
new_keybindings: &KeyBindings, new_keybindings: &KeyBindings,
) -> KeyBindings { ) -> KeyBindings {
for (mode, bindings) in new_keybindings.iter() { for (action, binding) in new_keybindings.iter() {
for (action, binding) in bindings { keybindings.insert(action.clone(), binding.clone());
match keybindings.get_mut(mode) {
Some(mode_bindings) => {
mode_bindings.insert(action.clone(), binding.clone());
}
None => {
keybindings.insert(
*mode,
[(action.clone(), binding.clone())]
.iter()
.cloned()
.collect(),
);
}
}
}
} }
keybindings keybindings
} }
@ -89,43 +90,30 @@ impl<'de> Deserialize<'de> for KeyBindings {
where where
D: Deserializer<'de>, D: Deserializer<'de>,
{ {
let parsed_map = FxHashMap::< let parsed_map =
Mode, FxHashMap::<Action, SerializedBinding>::deserialize(deserializer)?;
FxHashMap<Action, SerializedBinding>,
>::deserialize(deserializer)?;
let keybindings: FxHashMap<Mode, FxHashMap<Action, Binding>> = let keybindings: FxHashMap<Action, Binding> = parsed_map
parsed_map .into_iter()
.into_iter() .map(|(cmd, binding)| {
.map(|(mode, inner_map)| { (
let converted_inner_map = inner_map cmd,
.into_iter() match binding {
.map(|(cmd, binding)| { SerializedBinding::SingleKey(key_str) => {
( Binding::SingleKey(parse_key(&key_str).unwrap())
cmd, }
match binding { SerializedBinding::MultipleKeys(keys_str) => {
SerializedBinding::SingleKey(key_str) => { Binding::MultipleKeys(
Binding::SingleKey( keys_str
parse_key(&key_str).unwrap(), .iter()
) .map(|key_str| parse_key(key_str).unwrap())
} .collect(),
SerializedBinding::MultipleKeys(
keys_str,
) => Binding::MultipleKeys(
keys_str
.iter()
.map(|key_str| {
parse_key(key_str).unwrap()
})
.collect(),
),
},
) )
}) }
.collect(); },
(mode, converted_inner_map) )
}) })
.collect(); .collect();
Ok(KeyBindings(keybindings)) Ok(KeyBindings(keybindings))
} }
@ -380,4 +368,127 @@ mod tests {
KeyEvent::new(KeyCode::Enter, KeyModifiers::ALT) KeyEvent::new(KeyCode::Enter, KeyModifiers::ALT)
); );
} }
#[test]
fn test_deserialize_keybindings() {
let keybindings: KeyBindings = toml::from_str(
r#"
# Quit the application
quit = ["esc", "ctrl-c"]
# Scrolling through entries
select_next_entry = ["down", "ctrl-n", "ctrl-j"]
select_prev_entry = ["up", "ctrl-p", "ctrl-k"]
select_next_page = "pagedown"
select_prev_page = "pageup"
# Scrolling the preview pane
scroll_preview_half_page_down = "ctrl-d"
scroll_preview_half_page_up = "ctrl-u"
# Add entry to selection and move to the next entry
toggle_selection_down = "tab"
# Add entry to selection and move to the previous entry
toggle_selection_up = "backtab"
# Confirm selection
confirm_selection = "enter"
# Copy the selected entry to the clipboard
copy_entry_to_clipboard = "ctrl-y"
# Toggle the remote control mode
toggle_remote_control = "ctrl-r"
# Toggle the send to channel mode
toggle_send_to_channel = "ctrl-s"
# Toggle the help bar
toggle_help = "ctrl-g"
# Toggle the preview panel
toggle_preview = "ctrl-o"
"#,
)
.unwrap();
assert_eq!(
keybindings,
KeyBindings::from(vec![
(
Action::Quit,
Binding::MultipleKeys(vec![Key::Esc, Key::Ctrl('c'),])
),
(
Action::SelectNextEntry,
Binding::MultipleKeys(vec![
Key::Down,
Key::Ctrl('n'),
Key::Ctrl('j'),
])
),
(
Action::SelectPrevEntry,
Binding::MultipleKeys(vec![
Key::Up,
Key::Ctrl('p'),
Key::Ctrl('k'),
])
),
(Action::SelectNextPage, Binding::SingleKey(Key::PageDown)),
(Action::SelectPrevPage, Binding::SingleKey(Key::PageUp)),
(
Action::ScrollPreviewHalfPageDown,
Binding::SingleKey(Key::Ctrl('d'))
),
(
Action::ScrollPreviewHalfPageUp,
Binding::SingleKey(Key::Ctrl('u'))
),
(Action::ToggleSelectionDown, Binding::SingleKey(Key::Tab)),
(Action::ToggleSelectionUp, Binding::SingleKey(Key::BackTab)),
(Action::ConfirmSelection, Binding::SingleKey(Key::Enter)),
(
Action::CopyEntryToClipboard,
Binding::SingleKey(Key::Ctrl('y'))
),
(
Action::ToggleRemoteControl,
Binding::SingleKey(Key::Ctrl('r'))
),
(
Action::ToggleSendToChannel,
Binding::SingleKey(Key::Ctrl('s'))
),
(Action::ToggleHelp, Binding::SingleKey(Key::Ctrl('g'))),
(Action::TogglePreview, Binding::SingleKey(Key::Ctrl('o'))),
])
);
}
#[test]
fn test_merge_keybindings() {
let base_keybindings = KeyBindings::from(vec![
(Action::Quit, Binding::SingleKey(Key::Esc)),
(
Action::SelectNextEntry,
Binding::MultipleKeys(vec![Key::Down, Key::Ctrl('n')]),
),
(Action::SelectPrevEntry, Binding::SingleKey(Key::Up)),
]);
let custom_keybindings = KeyBindings::from(vec![
(Action::SelectNextEntry, Binding::SingleKey(Key::Ctrl('j'))),
(
Action::SelectPrevEntry,
Binding::MultipleKeys(vec![Key::Up, Key::Ctrl('k')]),
),
(Action::SelectNextPage, Binding::SingleKey(Key::PageDown)),
]);
let merged = merge_keybindings(base_keybindings, &custom_keybindings);
assert_eq!(
merged,
KeyBindings::from(vec![
(Action::Quit, Binding::SingleKey(Key::Esc)),
(Action::SelectNextEntry, Binding::SingleKey(Key::Ctrl('j'))),
(
Action::SelectPrevEntry,
Binding::MultipleKeys(vec![Key::Up, Key::Ctrl('k')]),
),
(Action::SelectNextPage, Binding::SingleKey(Key::PageDown)),
])
);
}
} }

View File

@ -7,7 +7,7 @@ use std::{
use anyhow::{Context, Result}; use anyhow::{Context, Result};
use directories::ProjectDirs; use directories::ProjectDirs;
use keybindings::merge_keybindings; pub use keybindings::merge_keybindings;
pub use keybindings::{parse_key, Binding, KeyBindings}; pub use keybindings::{parse_key, Binding, KeyBindings};
use previewers::PreviewersConfig; use previewers::PreviewersConfig;
use serde::Deserialize; use serde::Deserialize;
@ -100,6 +100,17 @@ pub fn default_config_from_file() -> Result<Config> {
Ok(default_config) Ok(default_config)
} }
const USER_CONFIG_ERROR_MSG: &str = "
If this follows a recent update, it is likely due to a breaking change in
the configuration format.
Check https://github.com/alexpasmantier/television/releases/latest for the ║
latest release notes.
";
impl Config { impl Config {
#[allow(clippy::missing_panics_doc, clippy::missing_errors_doc)] #[allow(clippy::missing_panics_doc, clippy::missing_errors_doc)]
pub fn new(config_env: &ConfigEnv) -> Result<Self> { pub fn new(config_env: &ConfigEnv) -> Result<Self> {
@ -134,8 +145,11 @@ impl Config {
fn load_user_config(config_dir: &Path) -> Result<Self> { fn load_user_config(config_dir: &Path) -> Result<Self> {
let path = config_dir.join(CONFIG_FILE_NAME); let path = config_dir.join(CONFIG_FILE_NAME);
let contents = std::fs::read_to_string(&path)?; let contents = std::fs::read_to_string(&path)?;
let user_cfg: Config = toml::from_str(&contents) let user_cfg: Config = toml::from_str(&contents).context(format!(
.context("Error parsing configuration file.")?; "Error parsing configuration file: {}\n{}",
path.display(),
USER_CONFIG_ERROR_MSG,
))?;
Ok(user_cfg) Ok(user_cfg)
} }
@ -246,7 +260,6 @@ fn default_tick_rate() -> f64 {
mod tests { mod tests {
use crate::action::Action; use crate::action::Action;
use crate::event::Key; use crate::event::Key;
use crate::television::Mode;
use super::*; use super::*;
use rustc_hash::FxHashMap; use rustc_hash::FxHashMap;
@ -322,11 +335,9 @@ mod tests {
[previewers.file] [previewers.file]
theme = "Visual Studio Dark" theme = "Visual Studio Dark"
[keybindings.Channel] [keybindings]
toggle_help = ["ctrl-a", "ctrl-b"] toggle_help = ["ctrl-a", "ctrl-b"]
confirm_selection = "ctrl-enter"
[keybindings.RemoteControl]
toggle_help = ["ctrl-c", "ctrl-d"]
[shell_integration.commands] [shell_integration.commands]
"git add" = "git-diff" "git add" = "git-diff"
@ -358,36 +369,18 @@ mod tests {
default_config.ui.theme = "television".to_string(); default_config.ui.theme = "television".to_string();
default_config.previewers.file.theme = default_config.previewers.file.theme =
"Visual Studio Dark".to_string(); "Visual Studio Dark".to_string();
default_config default_config.keybindings.extend({
.keybindings let mut map = FxHashMap::default();
.get_mut(&Mode::Channel) map.insert(
.unwrap() Action::ToggleHelp,
.extend({ Binding::MultipleKeys(vec![Key::Ctrl('a'), Key::Ctrl('b')]),
let mut map = FxHashMap::default(); );
map.insert( map.insert(
Action::ToggleHelp, Action::ConfirmSelection,
Binding::MultipleKeys(vec![ Binding::SingleKey(Key::CtrlEnter),
Key::Ctrl('a'), );
Key::Ctrl('b'), map
]), });
);
map
});
default_config
.keybindings
.get_mut(&Mode::RemoteControl)
.unwrap()
.extend({
let mut map = FxHashMap::default();
map.insert(
Action::ToggleHelp,
Binding::MultipleKeys(vec![
Key::Ctrl('c'),
Key::Ctrl('d'),
]),
);
map
});
default_config default_config
.shell_integration .shell_integration

View File

@ -193,16 +193,12 @@ pub fn draw(ctx: &Ctx, f: &mut Frame<'_>, area: Rect) -> Result<Layout> {
&ctx.colorscheme, &ctx.colorscheme,
&ctx.config &ctx.config
.keybindings .keybindings
.get(&ctx.tv_state.mode)
.unwrap()
.get(&Action::ToggleHelp) .get(&Action::ToggleHelp)
// just display the first keybinding // just display the first keybinding
.unwrap() .unwrap()
.to_string(), .to_string(),
&ctx.config &ctx.config
.keybindings .keybindings
.get(&ctx.tv_state.mode)
.unwrap()
.get(&Action::TogglePreview) .get(&Action::TogglePreview)
// just display the first keybinding // just display the first keybinding
.unwrap() .unwrap()

View File

@ -1,8 +1,6 @@
use rustc_hash::FxHashMap; use rustc_hash::FxHashMap;
use std::ops::Deref; use std::ops::Deref;
use crate::television::Mode;
use crate::action::Action; use crate::action::Action;
use crate::config::{Binding, KeyBindings}; use crate::config::{Binding, KeyBindings};
use crate::event::Key; use crate::event::Key;
@ -13,20 +11,15 @@ use crate::event::Key;
/// # Example: /// # Example:
/// ```ignore /// ```ignore
/// Keymap { /// Keymap {
/// Mode::Channel => { /// Key::Char('j') => Action::MoveDown,
/// Key::Char('j') => Action::MoveDown, /// Key::Char('k') => Action::MoveUp,
/// Key::Char('k') => Action::MoveUp, /// Key::Char('q') => Action::Quit,
/// Key::Char('q') => Action::Quit,
/// },
/// Mode::Insert => {
/// Key::Ctrl('a') => Action::MoveToStart,
/// },
/// } /// }
/// ``` /// ```
pub struct Keymap(pub FxHashMap<Mode, FxHashMap<Key, Action>>); pub struct Keymap(pub FxHashMap<Key, Action>);
impl Deref for Keymap { impl Deref for Keymap {
type Target = FxHashMap<Mode, FxHashMap<Key, Action>>; type Target = FxHashMap<Key, Action>;
fn deref(&self) -> &Self::Target { fn deref(&self) -> &Self::Target {
&self.0 &self.0
} }
@ -40,38 +33,18 @@ impl From<&KeyBindings> for Keymap {
/// key events. /// key events.
fn from(keybindings: &KeyBindings) -> Self { fn from(keybindings: &KeyBindings) -> Self {
let mut keymap = FxHashMap::default(); let mut keymap = FxHashMap::default();
for (mode, bindings) in keybindings.iter() { for (action, binding) in keybindings.iter() {
let mut mode_keymap = FxHashMap::default(); match binding {
for (action, binding) in bindings { Binding::SingleKey(key) => {
match binding { keymap.insert(*key, action.clone());
Binding::SingleKey(key) => { }
mode_keymap.insert(*key, action.clone()); Binding::MultipleKeys(keys) => {
} for key in keys {
Binding::MultipleKeys(keys) => { keymap.insert(*key, action.clone());
for key in keys {
mode_keymap.insert(*key, action.clone());
}
} }
} }
} }
keymap.insert(*mode, mode_keymap);
} }
Self(keymap) Self(keymap)
} }
} }
impl Keymap {
/// For a provided `Mode`, merge the given `mappings` into the keymap.
pub fn with_mode_mappings(
mut self,
mode: Mode,
mappings: Vec<(Key, Action)>,
) -> Self {
let mode_keymap = self.0.entry(mode).or_default();
for (key, action) in mappings {
mode_keymap.insert(key, action);
}
self
}
}

View File

@ -146,15 +146,7 @@ fn serialized_keys_for_actions(
) -> Vec<String> { ) -> Vec<String> {
actions actions
.iter() .iter()
.map(|a| { .map(|a| keybindings.get(a).unwrap().clone().to_string())
keybindings
.get(&Mode::Channel)
.unwrap()
.get(a)
.unwrap()
.clone()
.to_string()
})
.collect() .collect()
} }