From 66e2e72e4e66893af88de8aabdba306ac3f39be2 Mon Sep 17 00:00:00 2001 From: lalvarezt Date: Fri, 25 Jul 2025 10:46:29 +0200 Subject: [PATCH] refactor: cleanup or useless functions and improved config flags movement --- benches/main/ui.rs | 7 +-- television/app.rs | 8 +--- television/config/keybindings.rs | 6 ++- television/config/mod.rs | 19 ++++---- television/main.rs | 9 +--- television/screen/help_panel.rs | 2 +- television/screen/keybindings.rs | 31 +------------ television/television.rs | 74 ++++++++++---------------------- tests/app.rs | 8 +++- 9 files changed, 47 insertions(+), 117 deletions(-) diff --git a/benches/main/ui.rs b/benches/main/ui.rs index 984a623..8e095f4 100644 --- a/benches/main/ui.rs +++ b/benches/main/ui.rs @@ -485,13 +485,8 @@ pub fn draw(c: &mut Criterion) { tx, channel_prototype, config, - None, - false, - false, - Some(50), - false, cable.clone(), - &PostProcessedCli::default(), + PostProcessedCli::default(), ); tv.find("television"); for _ in 0..5 { diff --git a/television/app.rs b/television/app.rs index ee6c61c..33c2205 100644 --- a/television/app.rs +++ b/television/app.rs @@ -179,7 +179,6 @@ impl App { pub fn new( channel_prototype: ChannelPrototype, config: Config, - input: Option, options: AppOptions, cable_channels: Cable, cli_args: &PostProcessedCli, @@ -194,13 +193,8 @@ impl App { action_tx.clone(), channel_prototype, config, - input, - options.no_remote, - options.no_preview, - options.preview_size, - options.exact, cable_channels, - cli_args, + cli_args.clone(), ); // Create input map from the merged config that includes both key and event bindings diff --git a/television/config/keybindings.rs b/television/config/keybindings.rs index 2e92686..1cb6f79 100644 --- a/television/config/keybindings.rs +++ b/television/config/keybindings.rs @@ -288,7 +288,7 @@ where /// /// # Returns /// -/// The merged bindings collection +/// A new `Bindings` collection with merged key mappings. /// /// # Examples /// @@ -308,7 +308,9 @@ where /// ]); /// /// let merged = merge_bindings(base, &custom); -/// assert_eq!(merged.len(), 3); +/// assert_eq!(merged.get(&Key::Enter), Some(&Action::ConfirmSelection.into())); +/// assert_eq!(merged.get(&Key::Esc), Some(&Action::NoOp.into())); +/// assert_eq!(merged.get(&Key::Tab), Some(&Action::ToggleSelectionDown.into())); /// ``` pub fn merge_bindings( mut bindings: Bindings, diff --git a/television/config/mod.rs b/television/config/mod.rs index 30d996a..47c4654 100644 --- a/television/config/mod.rs +++ b/television/config/mod.rs @@ -1,8 +1,11 @@ use crate::{ + action::Action, cable::CABLE_DIR_NAME, channels::prototypes::{DEFAULT_PROTOTYPE_NAME, Template, UiSpec}, cli::PostProcessedCli, + features::FeatureFlags, history::DEFAULT_HISTORY_SIZE, + screen::keybindings::remove_action_bindings, }; use anyhow::{Context, Result}; use directories::ProjectDirs; @@ -222,10 +225,14 @@ impl Config { merged_keybindings.extend(new.shell_integration.keybindings.clone()); new.shell_integration.keybindings = merged_keybindings; - new.keybindings = + // merge keybindings with default keybindings + let keybindings = merge_bindings(default.keybindings.clone(), &new.keybindings); + new.keybindings = keybindings; - new.events = merge_bindings(default.events.clone(), &new.events); + // merge event bindings with default event bindings + let events = merge_bindings(default.events.clone(), &new.events); + new.events = events; Config { application: new.application, @@ -261,12 +268,6 @@ impl Config { /// Apply CLI overrides to this config pub fn apply_cli_overrides(&mut self, args: &PostProcessedCli) { - use crate::{ - action::Action, features::FeatureFlags, - screen::keybindings::remove_action_bindings, - }; - use tracing::debug; - debug!("Applying CLI overrides to config after channel merging"); if let Some(cable_dir) = &args.cable_dir { @@ -334,7 +335,7 @@ impl Config { self.ui.features.enable(FeatureFlags::HelpPanel); } - // Apply CLI keybinding overrides with proper source tracking + // Apply CLI keybinding overrides if let Some(keybindings) = &args.keybindings { self.apply_cli_keybinding_overrides(keybindings); } diff --git a/television/main.rs b/television/main.rs index a4cc1bd..000067f 100644 --- a/television/main.rs +++ b/television/main.rs @@ -95,14 +95,7 @@ async fn main() -> Result<()> { args.width, args.inline, ); - let mut app = App::new( - channel_prototype, - config, - args.input.clone(), - options, - cable, - &args, - ); + let mut app = App::new(channel_prototype, config, options, cable, &args); // If the user requested to show the remote control on startup, switch the // television into Remote Control mode before the application event loop diff --git a/television/screen/help_panel.rs b/television/screen/help_panel.rs index 9fdc5a9..b429a00 100644 --- a/television/screen/help_panel.rs +++ b/television/screen/help_panel.rs @@ -194,7 +194,7 @@ fn add_keybinding_lines_for_keys( } } -/// Generates the help content from the active bound keys +/// Generates the help content organized into global and mode-specific groups fn generate_help_content( config: &Config, mode: Mode, diff --git a/television/screen/keybindings.rs b/television/screen/keybindings.rs index 3f0db33..38652d1 100644 --- a/television/screen/keybindings.rs +++ b/television/screen/keybindings.rs @@ -3,7 +3,7 @@ use crate::{ config::KeyBindings, }; -/// Extract keys for a single action from the keybindings format +/// Extract keys for a single action pub fn find_keys_for_single_action( keybindings: &KeyBindings, target_action: &Action, @@ -22,35 +22,6 @@ pub fn find_keys_for_single_action( .collect() } -/// Extract keys for multiple actions and return them as a flat vector -pub fn extract_keys_for_actions( - keybindings: &KeyBindings, - actions: &[Actions], -) -> Vec { - actions - .iter() - .flat_map(|action| find_keys_for_action(keybindings, action)) - .collect() -} - -/// Extract keys for a single action from the new Key->Action keybindings format -pub fn find_keys_for_action( - keybindings: &KeyBindings, - target_action: &Actions, -) -> Vec { - keybindings - .bindings - .iter() - .filter_map(|(key, action)| { - if action == target_action { - Some(key.to_string()) - } else { - None - } - }) - .collect() -} - /// Remove all keybindings for a specific action from `KeyBindings` pub fn remove_action_bindings( keybindings: &mut KeyBindings, diff --git a/television/television.rs b/television/television.rs index 97a2534..ab52ee4 100644 --- a/television/television.rs +++ b/television/television.rs @@ -21,7 +21,6 @@ use crate::{ render::UiState, screen::{ colors::Colorscheme, - keybindings::remove_action_bindings, layout::InputPosition, spinner::{Spinner, SpinnerState}, }, @@ -80,8 +79,7 @@ pub struct Television { pub colorscheme: Colorscheme, pub ticks: u64, pub ui_state: UiState, - pub no_preview: bool, - pub preview_size: Option, + pub cli_args: PostProcessedCli, pub current_command_index: usize, pub channel_prototype: ChannelPrototype, } @@ -94,13 +92,8 @@ impl Television { action_tx: UnboundedSender, channel_prototype: ChannelPrototype, base_config: Config, - input: Option, - no_remote: bool, - no_preview: bool, - preview_size: Option, - exact: bool, cable_channels: Cable, - cli_args: &PostProcessedCli, + cli_args: PostProcessedCli, ) -> Self { let mut config = Self::merge_base_config_with_prototype_specs( &base_config, @@ -108,10 +101,15 @@ impl Television { ); // Apply ALL CLI overrides (including keybindings) after channel merging - config.apply_cli_overrides(cli_args); + config.apply_cli_overrides(&cli_args); debug!("Merged config: {:?}", config); + // Extract CLI arguments + let input = cli_args.input.clone(); + let exact = cli_args.exact; + let no_remote = cli_args.no_remote; + let mut results_picker = Picker::new(input.clone()); if config.ui.input_bar.position == InputPosition::Bottom { results_picker = results_picker.inverted(); @@ -189,8 +187,7 @@ impl Television { colorscheme, ticks: 0, ui_state: UiState::default(), - no_preview, - preview_size, + cli_args, current_command_index: 0, channel_prototype, } @@ -233,28 +230,6 @@ impl Television { config } - /// Apply CLI overrides to ensure they take precedence over channel prototype settings - fn apply_cli_overrides( - config: &mut Config, - no_preview: bool, - preview_size: Option, - ) { - // Handle preview panel flags - this mirrors the logic in main.rs but only for the subset - // of flags that Television manages directly - if no_preview { - config.ui.features.disable(FeatureFlags::PreviewPanel); - remove_action_bindings( - &mut config.keybindings, - &Action::TogglePreview.into(), - ); - } - - // Apply preview size regardless of preview state - if let Some(ps) = preview_size { - config.ui.preview_panel.size = ps; - } - } - fn setup_previewer( channel_prototype: &ChannelPrototype, ) -> Option<(UnboundedSender, UnboundedReceiver)> @@ -333,11 +308,7 @@ impl Television { &channel_prototype, ); // Reapply CLI overrides to ensure they persist across channel changes - Self::apply_cli_overrides( - &mut self.config, - self.no_preview, - self.preview_size, - ); + self.config.apply_cli_overrides(&self.cli_args); // Set preview state enabled based on both channel capability and UI configuration self.preview_state.enabled = channel_prototype.preview.is_some() && self @@ -952,21 +923,20 @@ mod test { let prototype = crate::channels::prototypes::ChannelPrototype::new( "test", "echo 1", ); + let cli_args = PostProcessedCli { + no_remote: true, + exact: true, + ..PostProcessedCli::default() + }; let tv = Television::new( tokio::sync::mpsc::unbounded_channel().0, prototype, config.clone(), - None, - true, - false, - Some(50), - true, Cable::from_prototypes(vec![]), - &PostProcessedCli::default(), + cli_args, ); assert_eq!(tv.matching_mode, MatchingMode::Substring); - assert!(!tv.no_preview); assert!(tv.remote_control.is_none()); } @@ -994,17 +964,17 @@ mod test { ) .unwrap(); + let cli_args = PostProcessedCli { + no_remote: true, + exact: true, + ..PostProcessedCli::default() + }; let tv = Television::new( tokio::sync::mpsc::unbounded_channel().0, prototype, config.clone(), - None, - true, - false, - Some(50), - true, Cable::from_prototypes(vec![]), - &PostProcessedCli::default(), + cli_args, ); assert_eq!( diff --git a/tests/app.rs b/tests/app.rs index 0d8d933..5dc678b 100644 --- a/tests/app.rs +++ b/tests/app.rs @@ -60,17 +60,21 @@ fn setup_app( None, false, ); + let cli_args = PostProcessedCli { + exact, + input: input.clone(), + ..PostProcessedCli::default() + }; let mut app = App::new( chan, config, - input, options, Cable::from_prototypes(vec![ ChannelPrototype::new("files", "find . -type f"), ChannelPrototype::new("dirs", "find . -type d"), ChannelPrototype::new("env", "printenv"), ]), - &PostProcessedCli::default(), + &cli_args, ); // retrieve the app's action channel handle in order to send a quit action