mirror of
https://github.com/alexpasmantier/television.git
synced 2025-07-29 06:11:37 +00:00
refactor(bindings): drop origin tracking for bindings, it creates more problems that it solves
This commit is contained in:
parent
38e4f0f03d
commit
112aa854d0
@ -3,7 +3,7 @@ use crate::{
|
||||
event::{Key, convert_raw_event_to_key},
|
||||
};
|
||||
use crossterm::event::{KeyCode, KeyEvent, KeyModifiers};
|
||||
use rustc_hash::{FxHashMap, FxHashSet};
|
||||
use rustc_hash::FxHashMap;
|
||||
use serde::{Deserialize, Serialize};
|
||||
use std::fmt::Display;
|
||||
use std::hash::Hash;
|
||||
@ -279,119 +279,21 @@ where
|
||||
}
|
||||
}
|
||||
|
||||
/// Represents the source type of keybindings being merged.
|
||||
///
|
||||
/// # Variants
|
||||
///
|
||||
/// * `Global` - Bindings from user configuration files or default settings
|
||||
/// * `Channel` - Bindings specific to individual channels/data sources
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
pub enum BindingSource {
|
||||
Global,
|
||||
Channel,
|
||||
}
|
||||
|
||||
impl BindingSource {
|
||||
/// Returns the string representation of the binding source.
|
||||
pub fn as_str(&self) -> &'static str {
|
||||
match self {
|
||||
BindingSource::Global => "global",
|
||||
BindingSource::Channel => "channel",
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Tracks source information for merged keybindings.
|
||||
#[derive(Debug, Clone, Default, PartialEq)]
|
||||
pub struct KeybindingSource {
|
||||
/// Keys that come from channel-specific configuration
|
||||
pub channel_keys: FxHashSet<Key>,
|
||||
/// Keys that come from global configuration
|
||||
pub global_keys: FxHashSet<Key>,
|
||||
}
|
||||
|
||||
impl KeybindingSource {
|
||||
/// Check if a key comes from channel configuration
|
||||
pub fn is_channel_key(&self, key: &Key) -> bool {
|
||||
self.channel_keys.contains(key)
|
||||
}
|
||||
|
||||
/// Check if a key comes from global configuration
|
||||
pub fn is_global_key(&self, key: &Key) -> bool {
|
||||
self.global_keys.contains(key)
|
||||
}
|
||||
|
||||
/// Add a key as coming from channel configuration
|
||||
pub fn add_channel_key(&mut self, key: Key) {
|
||||
self.channel_keys.insert(key);
|
||||
}
|
||||
|
||||
/// Add a key as coming from global configuration
|
||||
pub fn add_global_key(&mut self, key: Key) {
|
||||
self.global_keys.insert(key);
|
||||
}
|
||||
|
||||
/// Check if there are any tracked keybindings
|
||||
pub fn has_any_keys(&self) -> bool {
|
||||
!self.channel_keys.is_empty() || !self.global_keys.is_empty()
|
||||
}
|
||||
|
||||
/// Check if there are any channel-specific keybindings
|
||||
pub fn has_channel_keys(&self) -> bool {
|
||||
!self.channel_keys.is_empty()
|
||||
}
|
||||
|
||||
/// Check if there are any global keybindings
|
||||
pub fn has_global_keys(&self) -> bool {
|
||||
!self.global_keys.is_empty()
|
||||
}
|
||||
|
||||
/// Get the total count of tracked keys
|
||||
pub fn total_keys(&self) -> usize {
|
||||
self.channel_keys.len() + self.global_keys.len()
|
||||
}
|
||||
|
||||
/// Clear all tracking information
|
||||
pub fn clear(&mut self) {
|
||||
self.channel_keys.clear();
|
||||
self.global_keys.clear();
|
||||
}
|
||||
}
|
||||
|
||||
impl Hash for KeybindingSource {
|
||||
fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
|
||||
let mut channel_sorted: Vec<_> = self.channel_keys.iter().collect();
|
||||
let mut global_sorted: Vec<_> = self.global_keys.iter().collect();
|
||||
|
||||
channel_sorted.sort(); // Key must implement Ord
|
||||
global_sorted.sort();
|
||||
|
||||
for key in channel_sorted {
|
||||
key.hash(state);
|
||||
}
|
||||
for key in global_sorted {
|
||||
key.hash(state);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Merges two binding collections, with new bindings taking precedence.
|
||||
/// Keeps track of the source information.
|
||||
///
|
||||
/// # Arguments
|
||||
///
|
||||
/// * `bindings` - The base bindings collection (will be consumed)
|
||||
/// * `new_bindings` - The new bindings to merge in (higher precedence)
|
||||
/// * `source` - The source type of the new bindings being merged
|
||||
///
|
||||
/// # Returns
|
||||
///
|
||||
/// A tuple of (`merged_bindings`, `source_tracking`)
|
||||
/// The merged bindings collection
|
||||
///
|
||||
/// # Examples
|
||||
///
|
||||
/// ```rust
|
||||
/// use television::config::keybindings::{KeyBindings, merge_bindings, BindingSource};
|
||||
/// use television::config::keybindings::{KeyBindings, merge_bindings};
|
||||
/// use television::event::Key;
|
||||
/// use television::action::Action;
|
||||
///
|
||||
@ -405,53 +307,27 @@ impl Hash for KeybindingSource {
|
||||
/// (Key::Tab, Action::ToggleSelectionDown), // Add new binding
|
||||
/// ]);
|
||||
///
|
||||
/// let (merged, source) = merge_bindings(base, &custom, BindingSource::Channel);
|
||||
/// assert!(source.is_channel_key(&Key::Esc));
|
||||
/// assert!(source.is_channel_key(&Key::Tab));
|
||||
/// assert!(source.is_global_key(&Key::Enter));
|
||||
/// let merged = merge_bindings(base, &custom);
|
||||
/// assert_eq!(merged.len(), 3);
|
||||
/// ```
|
||||
pub fn merge_bindings<K>(
|
||||
mut bindings: Bindings<K>,
|
||||
new_bindings: &Bindings<K>,
|
||||
source: BindingSource,
|
||||
) -> (Bindings<K>, KeybindingSource)
|
||||
) -> Bindings<K>
|
||||
where
|
||||
K: Display + FromStr + Clone + Eq + Hash + std::fmt::Debug,
|
||||
K::Err: Display,
|
||||
{
|
||||
let mut keybinding_source = KeybindingSource::default();
|
||||
|
||||
debug!("bindings before: {:?}", bindings.bindings);
|
||||
|
||||
// Mark existing keys as global
|
||||
for key in bindings.bindings.keys() {
|
||||
if let Ok(tv_key) = Key::from_str(&key.to_string()) {
|
||||
keybinding_source.add_global_key(tv_key);
|
||||
}
|
||||
}
|
||||
|
||||
// Merge new bindings and mark them based on source type
|
||||
// Merge new bindings - they take precedence over existing ones
|
||||
for (key, actions) in &new_bindings.bindings {
|
||||
bindings.bindings.insert(key.clone(), actions.clone());
|
||||
|
||||
if let Ok(tv_key) = Key::from_str(&key.to_string()) {
|
||||
match source {
|
||||
BindingSource::Channel => {
|
||||
keybinding_source.add_channel_key(tv_key);
|
||||
// Remove from global keys if it was there (override)
|
||||
keybinding_source.global_keys.remove(&tv_key);
|
||||
}
|
||||
BindingSource::Global => {
|
||||
// New bindings are global - add to global keys
|
||||
keybinding_source.add_global_key(tv_key);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
debug!("bindings after: {:?}", bindings.bindings);
|
||||
|
||||
(bindings, keybinding_source)
|
||||
bindings
|
||||
}
|
||||
|
||||
impl Default for Bindings<Key> {
|
||||
@ -1140,11 +1016,7 @@ mod tests {
|
||||
(Key::PageDown, Action::SelectNextPage),
|
||||
]);
|
||||
|
||||
let (merged, _) = merge_bindings(
|
||||
base_keybindings,
|
||||
&custom_keybindings,
|
||||
BindingSource::Global,
|
||||
);
|
||||
let merged = merge_bindings(base_keybindings, &custom_keybindings);
|
||||
|
||||
// Should contain both base and custom keybindings
|
||||
assert!(merged.bindings.contains_key(&Key::Esc));
|
||||
@ -1255,11 +1127,7 @@ mod tests {
|
||||
bindings: custom_bindings,
|
||||
};
|
||||
|
||||
let (merged, _) = merge_bindings(
|
||||
base_keybindings,
|
||||
&custom_keybindings,
|
||||
BindingSource::Global,
|
||||
);
|
||||
let merged = merge_bindings(base_keybindings, &custom_keybindings);
|
||||
|
||||
// Custom multiple actions should be present
|
||||
assert_eq!(
|
||||
|
@ -2,7 +2,6 @@ use crate::{
|
||||
cable::CABLE_DIR_NAME,
|
||||
channels::prototypes::{DEFAULT_PROTOTYPE_NAME, Template, UiSpec},
|
||||
cli::PostProcessedCli,
|
||||
event::Key,
|
||||
history::DEFAULT_HISTORY_SIZE,
|
||||
};
|
||||
use anyhow::{Context, Result};
|
||||
@ -13,14 +12,10 @@ use std::{
|
||||
env,
|
||||
hash::Hash,
|
||||
path::{Path, PathBuf},
|
||||
str::FromStr,
|
||||
};
|
||||
use tracing::{debug, warn};
|
||||
|
||||
pub use keybindings::{
|
||||
BindingSource, EventBindings, EventType, KeyBindings, KeybindingSource,
|
||||
merge_bindings,
|
||||
};
|
||||
pub use keybindings::{EventBindings, EventType, KeyBindings, merge_bindings};
|
||||
pub use themes::Theme;
|
||||
pub use ui::UiConfig;
|
||||
|
||||
@ -96,9 +91,6 @@ pub struct Config {
|
||||
/// Shell integration configuration
|
||||
#[serde(default)]
|
||||
pub shell_integration: ShellIntegrationConfig,
|
||||
/// Keybinding source tracking (not serialized)
|
||||
#[serde(skip)]
|
||||
pub keybinding_source: KeybindingSource,
|
||||
}
|
||||
|
||||
const PROJECT_NAME: &str = "television";
|
||||
@ -230,20 +222,10 @@ impl Config {
|
||||
merged_keybindings.extend(new.shell_integration.keybindings.clone());
|
||||
new.shell_integration.keybindings = merged_keybindings;
|
||||
|
||||
let (keybindings, keybinding_source) = merge_bindings(
|
||||
default.keybindings.clone(),
|
||||
&new.keybindings,
|
||||
BindingSource::Global,
|
||||
);
|
||||
new.keybindings = keybindings;
|
||||
new.keybinding_source = keybinding_source;
|
||||
new.keybindings =
|
||||
merge_bindings(default.keybindings.clone(), &new.keybindings);
|
||||
|
||||
let (events, _) = merge_bindings(
|
||||
default.events.clone(),
|
||||
&new.events,
|
||||
BindingSource::Global,
|
||||
);
|
||||
new.events = events;
|
||||
new.events = merge_bindings(default.events.clone(), &new.events);
|
||||
|
||||
Config {
|
||||
application: new.application,
|
||||
@ -251,29 +233,14 @@ impl Config {
|
||||
events: new.events,
|
||||
ui: new.ui,
|
||||
shell_integration: new.shell_integration,
|
||||
keybinding_source: new.keybinding_source,
|
||||
}
|
||||
}
|
||||
|
||||
pub fn merge_channel_keybindings(&mut self, other: &KeyBindings) {
|
||||
let (merged, source) = merge_bindings(
|
||||
self.keybindings.clone(),
|
||||
other,
|
||||
BindingSource::Channel,
|
||||
);
|
||||
self.keybindings = merged;
|
||||
for key in &source.channel_keys {
|
||||
self.keybinding_source.add_channel_key(*key);
|
||||
self.keybinding_source.global_keys.remove(key);
|
||||
}
|
||||
self.keybindings = merge_bindings(self.keybindings.clone(), other);
|
||||
}
|
||||
|
||||
/// Apply CLI keybinding overrides while preserving source categorization.
|
||||
///
|
||||
/// CLI overrides preserve the original source of the key:
|
||||
/// - If overriding a global key → keep it in global list
|
||||
/// - If overriding a channel key → keep it in channel list
|
||||
/// - If adding a new key → add to global list (CLI is user's global preference)
|
||||
/// Apply CLI keybinding overrides.
|
||||
pub fn apply_cli_keybinding_overrides(
|
||||
&mut self,
|
||||
cli_keybindings: &KeyBindings,
|
||||
@ -281,26 +248,15 @@ impl Config {
|
||||
debug!("keybindings before: {:?}", self.keybindings);
|
||||
|
||||
for (key, actions) in &cli_keybindings.bindings {
|
||||
let was_existing = self.keybindings.contains_key(key);
|
||||
|
||||
// Update the keybinding
|
||||
self.keybindings.insert(*key, actions.clone());
|
||||
|
||||
if let Ok(tv_key) = Key::from_str(&key.to_string()) {
|
||||
if !was_existing {
|
||||
// New key from CLI - add to global (CLI is user's global preference)
|
||||
self.keybinding_source.add_global_key(tv_key);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
debug!("keybindings after: {:?}", self.keybindings);
|
||||
}
|
||||
|
||||
pub fn merge_event_bindings(&mut self, other: &EventBindings) {
|
||||
let (merged, _) =
|
||||
merge_bindings(self.events.clone(), other, BindingSource::Global);
|
||||
self.events = merged;
|
||||
self.events = merge_bindings(self.events.clone(), other);
|
||||
}
|
||||
|
||||
/// Apply CLI overrides to this config
|
||||
|
@ -1,7 +1,6 @@
|
||||
use crate::{
|
||||
action::Action,
|
||||
config::{Config, KeyBindings},
|
||||
event::Key,
|
||||
screen::colors::Colorscheme,
|
||||
television::Mode,
|
||||
};
|
||||
@ -12,6 +11,7 @@ use ratatui::{
|
||||
text::{Line, Span},
|
||||
widgets::{Block, BorderType, Borders, Clear, Padding, Paragraph},
|
||||
};
|
||||
use tracing::{debug, trace};
|
||||
|
||||
const MIN_PANEL_WIDTH: u16 = 25;
|
||||
const MIN_PANEL_HEIGHT: u16 = 5;
|
||||
@ -57,49 +57,31 @@ pub fn draw_help_panel(
|
||||
fn add_keybinding_lines_for_keys(
|
||||
lines: &mut Vec<Line<'static>>,
|
||||
keybindings: &KeyBindings,
|
||||
keys: impl Iterator<Item = Key>,
|
||||
mode: Mode,
|
||||
colorscheme: &Colorscheme,
|
||||
category_name: &str,
|
||||
) {
|
||||
use tracing::trace;
|
||||
|
||||
// Collect all valid keybinding entries
|
||||
let mut entries: Vec<(String, String)> = Vec::new();
|
||||
let mut filtered_count = 0;
|
||||
|
||||
for key in keys {
|
||||
if let Some(actions) = keybindings.get(&key) {
|
||||
for action in actions.as_slice() {
|
||||
// Filter out NoOp actions (unbound keys)
|
||||
if matches!(action, Action::NoOp) {
|
||||
trace!(
|
||||
"Filtering out NoOp (unboud keys) action for key '{}' in {} category",
|
||||
key, category_name
|
||||
);
|
||||
filtered_count += 1;
|
||||
continue;
|
||||
}
|
||||
|
||||
let description = action.description();
|
||||
let key_string = key.to_string();
|
||||
entries.push((description.to_string(), key_string.clone()));
|
||||
trace!(
|
||||
"Added keybinding: {} -> {} ({})",
|
||||
key_string, description, category_name
|
||||
);
|
||||
for (key, actions) in keybindings.iter() {
|
||||
for action in actions.as_slice() {
|
||||
// Filter out NoOp actions (unbound keys)
|
||||
continue;
|
||||
}
|
||||
|
||||
let description = action.description();
|
||||
let key_string = key.to_string();
|
||||
entries.push((description.to_string(), key_string.clone()));
|
||||
trace!(
|
||||
"Added keybinding: {} -> {} ({})",
|
||||
key_string, description, category_name
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
// Sort entries alphabetically by description
|
||||
entries.sort_by(|a, b| a.0.cmp(&b.0));
|
||||
trace!(
|
||||
"Sorted {} keybindings for {} category (filtered {} NoOp entries)",
|
||||
entries.len(),
|
||||
category_name,
|
||||
filtered_count
|
||||
);
|
||||
|
||||
// Create lines from sorted entries
|
||||
for (description, key_string) in entries {
|
||||
@ -112,22 +94,15 @@ fn add_keybinding_lines_for_keys(
|
||||
}
|
||||
}
|
||||
|
||||
/// Generates the help content organized into global and channel-specific groups
|
||||
/// Generates the help content from the active bound keys
|
||||
fn generate_help_content(
|
||||
config: &Config,
|
||||
mode: Mode,
|
||||
colorscheme: &Colorscheme,
|
||||
) -> Vec<Line<'static>> {
|
||||
use tracing::debug;
|
||||
|
||||
let mut lines = Vec::new();
|
||||
|
||||
debug!("Generating help content for mode: {:?}", mode);
|
||||
debug!(
|
||||
"Keybinding source tracking - Global keys: {}, Channel keys: {}",
|
||||
config.keybinding_source.global_keys.len(),
|
||||
config.keybinding_source.channel_keys.len()
|
||||
);
|
||||
|
||||
// Global keybindings section header
|
||||
lines.push(Line::from(vec![Span::styled(
|
||||
@ -138,67 +113,14 @@ fn generate_help_content(
|
||||
.underlined(),
|
||||
)]));
|
||||
|
||||
// Global keybindings - all keys that are NOT from channel configs
|
||||
let global_keys = config.keybinding_source.global_keys.iter().copied();
|
||||
add_keybinding_lines_for_keys(
|
||||
&mut lines,
|
||||
&config.keybindings,
|
||||
global_keys,
|
||||
mode,
|
||||
colorscheme,
|
||||
"Global",
|
||||
);
|
||||
|
||||
// Add spacing between Global and channel-specific sections only if we have channel keys
|
||||
if !config.keybinding_source.channel_keys.is_empty() {
|
||||
lines.push(Line::from(""));
|
||||
}
|
||||
|
||||
// Channel-specific keybindings section header
|
||||
let mode_name = match mode {
|
||||
Mode::Channel => "Channel",
|
||||
Mode::RemoteControl => "Remote",
|
||||
};
|
||||
|
||||
// Only show channel section if there are channel-specific keybindings
|
||||
if config.keybinding_source.has_channel_keys() {
|
||||
lines.push(Line::from(vec![Span::styled(
|
||||
mode_name,
|
||||
Style::default()
|
||||
.fg(colorscheme.help.metadata_field_name_fg)
|
||||
.bold()
|
||||
.underlined(),
|
||||
)]));
|
||||
|
||||
// Channel-specific keybindings - only keys from channel configs
|
||||
let channel_keys =
|
||||
config.keybinding_source.channel_keys.iter().copied();
|
||||
add_keybinding_lines_for_keys(
|
||||
&mut lines,
|
||||
&config.keybindings,
|
||||
channel_keys,
|
||||
mode,
|
||||
colorscheme,
|
||||
mode_name,
|
||||
);
|
||||
} else {
|
||||
debug!(
|
||||
"No channel-specific keybindings found, skipping channel section for mode: {:?}",
|
||||
mode
|
||||
);
|
||||
}
|
||||
|
||||
// Handle edge case where no keybindings are found at all
|
||||
if !config.keybinding_source.has_any_keys() {
|
||||
debug!("Warning: No keybindings found in source tracking!");
|
||||
lines.push(Line::from(vec![Span::styled(
|
||||
"No keybindings configured",
|
||||
Style::default()
|
||||
.fg(colorscheme.help.metadata_field_name_fg)
|
||||
.italic(),
|
||||
)]));
|
||||
}
|
||||
|
||||
debug!("Generated help content with {} total lines", lines.len());
|
||||
lines
|
||||
}
|
||||
@ -233,8 +155,6 @@ pub fn calculate_help_panel_size(
|
||||
mode: Mode,
|
||||
colorscheme: &Colorscheme,
|
||||
) -> (u16, u16) {
|
||||
use tracing::trace;
|
||||
|
||||
// Generate content to count items and calculate width
|
||||
let content = generate_help_content(config, mode, colorscheme);
|
||||
|
||||
|
Loading…
x
Reference in New Issue
Block a user