From 82e3f890c83eb435c6d0d118a7e72ac30dbb3059 Mon Sep 17 00:00:00 2001 From: Alexandre Pasmantier <47638216+alexpasmantier@users.noreply.github.com> Date: Sun, 6 Apr 2025 16:16:41 +0000 Subject: [PATCH] refactor(passthrough)!: drop support for unused passthrough keybindings (#446) BREAKING CHANGE: the CLI `passthrough_keybindings` are no longer available. This feature wasn't used anywhere to my knowledge (github search) and was adding unnecessary complexity to the code. --- television/action.rs | 4 ---- television/app.rs | 51 ++++++------------------------------------ television/cli/args.rs | 8 ------- television/cli/mod.rs | 32 ++++++++------------------ television/main.rs | 8 ++----- tests/app.rs | 3 +-- 6 files changed, 19 insertions(+), 87 deletions(-) diff --git a/television/action.rs b/television/action.rs index ee993d9..4b16afb 100644 --- a/television/action.rs +++ b/television/action.rs @@ -51,10 +51,6 @@ pub enum Action { #[serde(alias = "select_entry")] #[serde(alias = "confirm_selection")] ConfirmSelection, - /// Select the entry currently under the cursor and pass the key that was pressed - /// through to be handled the parent process. - #[serde(alias = "select_passthrough")] - SelectPassthrough(String), /// Select the entry currently under the cursor and exit the application. #[serde(alias = "select_and_exit")] SelectAndExit, diff --git a/television/app.rs b/television/app.rs index e1fa86e..b3613e1 100644 --- a/television/app.rs +++ b/television/app.rs @@ -4,11 +4,9 @@ use anyhow::Result; use tokio::sync::mpsc; use tracing::{debug, trace}; -use crate::channels::entry::Entry; +use crate::channels::entry::{Entry, PreviewType}; use crate::channels::TelevisionChannel; -use crate::config::{ - merge_keybindings, parse_key, Binding, Config, KeyBindings, -}; +use crate::config::Config; use crate::keymap::Keymap; use crate::render::UiState; use crate::television::{Mode, Television}; @@ -62,7 +60,6 @@ pub struct App { pub enum ActionOutcome { Entries(FxHashSet), Input(String), - Passthrough(FxHashSet, String), None, } @@ -70,7 +67,6 @@ pub enum ActionOutcome { #[derive(Debug)] pub struct AppOutput { pub selected_entries: Option>, - pub passthrough: Option, } impl From for AppOutput { @@ -78,19 +74,15 @@ impl From for AppOutput { match outcome { ActionOutcome::Entries(entries) => Self { selected_entries: Some(entries), - passthrough: None, }, ActionOutcome::Input(input) => Self { - selected_entries: None, - passthrough: Some(input), - }, - ActionOutcome::Passthrough(entries, key) => Self { - selected_entries: Some(entries), - passthrough: Some(key), + selected_entries: Some(FxHashSet::from_iter([Entry::new( + input, + PreviewType::None, + )])), }, ActionOutcome::None => Self { selected_entries: None, - passthrough: None, }, } } @@ -103,7 +95,6 @@ impl App { pub fn new( channel: TelevisionChannel, config: Config, - passthrough_keybindings: &[String], input: Option, ) -> Self { let (action_tx, action_rx) = mpsc::unbounded_channel(); @@ -111,21 +102,7 @@ impl App { let (_, event_rx) = mpsc::unbounded_channel(); let (event_abort_tx, _) = mpsc::unbounded_channel(); let tick_rate = config.application.tick_rate; - let keybindings = merge_keybindings(config.keybindings.clone(), { - &KeyBindings::from(passthrough_keybindings.iter().filter_map( - |s| match parse_key(s) { - Ok(key) => Some(( - Action::SelectPassthrough(s.to_string()), - Binding::SingleKey(key), - )), - Err(e) => { - debug!("Failed to parse keybinding: {}", e); - None - } - }, - )) - }); - let keymap = Keymap::from(&keybindings); + let keymap = Keymap::from(&config.keybindings); debug!("{:?}", keymap); let (ui_state_tx, ui_state_rx) = mpsc::unbounded_channel(); @@ -346,20 +323,6 @@ impl App { self.television.current_pattern.clone(), )); } - Action::SelectPassthrough(passthrough) => { - self.should_quit = true; - self.render_tx.send(RenderingTask::Quit)?; - if let Some(entries) = self - .television - .get_selected_entries(Some(Mode::Channel)) - { - return Ok(ActionOutcome::Passthrough( - entries, - passthrough, - )); - } - return Ok(ActionOutcome::None); - } Action::ClearScreen => { self.render_tx.send(RenderingTask::ClearScreen)?; } diff --git a/television/cli/args.rs b/television/cli/args.rs index 66cfee7..f014fb1 100644 --- a/television/cli/args.rs +++ b/television/cli/args.rs @@ -66,14 +66,6 @@ pub struct Cli { #[arg(short, long, value_name = "STRING", verbatim_doc_comment)] pub keybindings: Option, - /// Passthrough keybindings (comma separated, e.g. "q,ctrl-w,ctrl-t") - /// - /// These keybindings will trigger selection of the current entry and be - /// passed through to stdout along with the entry to be handled by the - /// parent process. - #[arg(long, value_name = "STRING", verbatim_doc_comment)] - pub passthrough_keybindings: Option, - /// Input text to pass to the channel to prefill the prompt. /// /// This can be used to provide a default value for the prompt upon diff --git a/television/cli/mod.rs b/television/cli/mod.rs index 91e7b38..02044f7 100644 --- a/television/cli/mod.rs +++ b/television/cli/mod.rs @@ -24,7 +24,6 @@ pub struct PostProcessedCli { pub no_preview: bool, pub tick_rate: Option, pub frame_rate: Option, - pub passthrough_keybindings: Vec, pub input: Option, pub command: Option, pub working_directory: Option, @@ -40,7 +39,6 @@ impl Default for PostProcessedCli { no_preview: false, tick_rate: None, frame_rate: None, - passthrough_keybindings: Vec::new(), input: None, command: None, working_directory: None, @@ -52,21 +50,16 @@ impl Default for PostProcessedCli { impl From for PostProcessedCli { fn from(cli: Cli) -> Self { + // parse literal keybindings passed through the CLI let keybindings: Option = cli.keybindings.map(|kb| { - parse_keybindings(&kb) + parse_keybindings_literal(&kb, CLI_KEYBINDINGS_DELIMITER) .map_err(|e| { cli_parsing_error_exit(&e.to_string()); }) .unwrap() }); - let passthrough_keybindings = cli - .passthrough_keybindings - .unwrap_or_default() - .split(',') - .map(std::string::ToString::to_string) - .collect(); - + // parse the preview command if provided let preview_kind = cli .preview .map(|preview| PreviewCommand { @@ -110,7 +103,6 @@ impl From for PostProcessedCli { no_preview: cli.no_preview, tick_rate: cli.tick_rate, frame_rate: cli.frame_rate, - passthrough_keybindings, input: cli.input, command: cli.command, working_directory, @@ -147,7 +139,7 @@ impl ParsedCliChannel { const CLI_KEYBINDINGS_DELIMITER: char = ';'; -/// Parse the keybindings string into a hashmap of key -> action. +/// Parse a keybindings literal into a `KeyBindings` struct. /// /// The formalism used is the same as the one used in the configuration file: /// ```ignore @@ -155,9 +147,12 @@ const CLI_KEYBINDINGS_DELIMITER: char = ';'; /// ``` /// Parsing it globally consists of splitting by the delimiter, reconstructing toml key-value pairs /// and parsing that using logic already implemented in the configuration module. -fn parse_keybindings(cli_keybindings: &str) -> Result { +fn parse_keybindings_literal( + cli_keybindings: &str, + delimiter: char, +) -> Result { let toml_definition = cli_keybindings - .split(CLI_KEYBINDINGS_DELIMITER) + .split(delimiter) .fold(String::new(), |acc, kb| acc + kb + "\n"); toml::from_str(&toml_definition).map_err(|e| anyhow!(e)) @@ -323,7 +318,6 @@ mod tests { tick_rate: Some(50.0), frame_rate: Some(60.0), keybindings: None, - passthrough_keybindings: Some("q,ctrl-w,ctrl-t".to_string()), input: None, command: None, working_directory: Some("/home/user".to_string()), @@ -345,10 +339,6 @@ mod tests { ); assert_eq!(post_processed_cli.tick_rate, Some(50.0)); assert_eq!(post_processed_cli.frame_rate, Some(60.0)); - assert_eq!( - post_processed_cli.passthrough_keybindings, - vec!["q".to_string(), "ctrl-w".to_string(), "ctrl-t".to_string()] - ); assert_eq!( post_processed_cli.working_directory, Some("/home/user".to_string()) @@ -366,7 +356,6 @@ mod tests { tick_rate: Some(50.0), frame_rate: Some(60.0), keybindings: None, - passthrough_keybindings: None, input: None, command: None, working_directory: None, @@ -396,7 +385,6 @@ mod tests { tick_rate: Some(50.0), frame_rate: Some(60.0), keybindings: None, - passthrough_keybindings: None, input: None, command: None, working_directory: None, @@ -421,7 +409,6 @@ mod tests { tick_rate: Some(50.0), frame_rate: Some(60.0), keybindings: None, - passthrough_keybindings: None, input: None, command: None, working_directory: None, @@ -449,7 +436,6 @@ mod tests { "quit=\"esc\";select_next_entry=[\"down\",\"ctrl-j\"]" .to_string(), ), - passthrough_keybindings: None, input: None, command: None, working_directory: None, diff --git a/television/main.rs b/television/main.rs index 94f9378..8a9eb5c 100644 --- a/television/main.rs +++ b/television/main.rs @@ -65,16 +65,12 @@ async fn main() -> Result<()> { CLIPBOARD.with(<_>::default); debug!("Creating application..."); - let mut app = - App::new(channel, config, &args.passthrough_keybindings, args.input); + let mut app = App::new(channel, config, args.input); stdout().flush()?; let output = app.run(stdout().is_terminal(), false).await?; - info!("{:?}", output); + info!("App output: {:?}", output); let stdout_handle = stdout().lock(); let mut bufwriter = BufWriter::new(stdout_handle); - if let Some(passthrough) = output.passthrough { - writeln!(bufwriter, "{passthrough}")?; - } if let Some(entries) = output.selected_entries { for entry in &entries { writeln!(bufwriter, "{}", entry.stdout_repr())?; diff --git a/tests/app.rs b/tests/app.rs index 547ec9e..ed9d81e 100644 --- a/tests/app.rs +++ b/tests/app.rs @@ -30,10 +30,9 @@ fn setup_app() -> ( television::channels::files::Channel::new(vec![target_dir]), ); let config = default_config_from_file().unwrap(); - let passthrough_keybindings = Vec::new(); let input = None; - let mut app = App::new(channel, config, &passthrough_keybindings, input); + let mut app = App::new(channel, config, input); // retrieve the app's action channel handle in order to send a quit action let tx = app.action_tx.clone();