From 1abf545ed953dcb9b26a7926df2df105be662c6f Mon Sep 17 00:00:00 2001 From: LoricAndre <57358788+LoricAndre@users.noreply.github.com> Date: Tue, 3 Dec 2024 11:21:35 +0100 Subject: [PATCH] fix: fix --tmux quoting (#643) * fix: fix --tmux quoting * fix e2e * use fifo for input * fix macos e2e --------- Co-authored-by: LoricAndre --- Cargo.lock | 21 +++++++++++++++++ e2e/tests/tmux.rs | 60 ++++++++++++++++++++++++++++++++++++++++++++++- skim/Cargo.toml | 3 ++- skim/src/item.rs | 4 ++++ skim/src/tmux.rs | 53 ++++++++++++++++++++++++++++++----------- 5 files changed, 125 insertions(+), 16 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index db3f0a02..b9cc28aa 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -116,6 +116,17 @@ version = "2.6.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b048fb63fd8b5923fc5aa7b340d8e156aec7ec02f0c78fa8a6ddc2613f6f71de" +[[package]] +name = "bstr" +version = "1.11.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1a68f1f47cdf0ec8ee4b941b2eee2a80cb796db73118c0dd09ac63fbe405be22" +dependencies = [ + "memchr", + "regex-automata", + "serde", +] + [[package]] name = "bumpalo" version = "3.16.0" @@ -844,6 +855,15 @@ dependencies = [ "syn", ] +[[package]] +name = "shell-quote" +version = "0.7.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ae4c63bdcc11eea49b562941b914d5ac30d42cad982e3f6e846a513ee6a3ce7e" +dependencies = [ + "bstr", +] + [[package]] name = "shlex" version = "1.3.0" @@ -871,6 +891,7 @@ dependencies = [ "rand", "rayon", "regex", + "shell-quote", "shlex", "time", "timer", diff --git a/e2e/tests/tmux.rs b/e2e/tests/tmux.rs index 7f45fef7..ba44c4f9 100644 --- a/e2e/tests/tmux.rs +++ b/e2e/tests/tmux.rs @@ -17,7 +17,7 @@ fn setup_tmux_mock(tmux: &TmuxController) -> Result { writer.write_fmt(format_args!( "#!/bin/sh -echo $@ > {} +echo \"$@\" > {} ", outfile.to_str().unwrap() ))?; @@ -63,3 +63,61 @@ fn tmux_stdin() -> Result<()> { Ok(()) } + +#[test] +fn tmux_quote_bash() -> Result<()> { + let tmux = TmuxController::new()?; + let outfile = setup_tmux_mock(&tmux)?; + tmux.send_keys(&[Str("export SHELL=/bin/bash"), Enter])?; + tmux.start_sk(None, &["--tmux", "--bind 'ctrl-a:reload(ls /foo*)'"])?; + tmux.until(|_| Path::new(&outfile).exists())?; + let cmd = get_tmux_cmd(&outfile)?; + assert!(cmd.starts_with("display-popup")); + assert!(cmd.contains("-E")); + assert!(cmd.contains("--bind $'ctrl-a:reload(ls /foo*)'")); + + Ok(()) +} +#[test] +fn tmux_quote_zsh() -> Result<()> { + let tmux = TmuxController::new()?; + let outfile = setup_tmux_mock(&tmux)?; + tmux.send_keys(&[Str("export SHELL=/bin/zsh"), Enter])?; + tmux.start_sk(None, &["--tmux", "--bind 'ctrl-a:reload(ls /foo*)'"])?; + tmux.until(|_| Path::new(&outfile).exists())?; + let cmd = get_tmux_cmd(&outfile)?; + println!("{cmd}"); + assert!(cmd.starts_with("display-popup")); + assert!(cmd.contains("-E")); + assert!(cmd.contains("sk --bind $'ctrl-a:reload(ls /foo*)' >")); + + Ok(()) +} +#[test] +fn tmux_quote_sh() -> Result<()> { + let tmux = TmuxController::new()?; + let outfile = setup_tmux_mock(&tmux)?; + tmux.send_keys(&[Str("export SHELL=/bin/sh"), Enter])?; + tmux.start_sk(None, &["--tmux", "--bind 'ctrl-a:reload(ls /foo*)'"])?; + tmux.until(|_| Path::new(&outfile).exists())?; + let cmd = get_tmux_cmd(&outfile)?; + assert!(cmd.starts_with("display-popup")); + assert!(cmd.contains("-E")); + assert!(cmd.contains("--bind ctrl-a':reload(ls /foo*)'")); + + Ok(()) +} +#[test] +fn tmux_quote_fish() -> Result<()> { + let tmux = TmuxController::new()?; + let outfile = setup_tmux_mock(&tmux)?; + tmux.send_keys(&[Str("export SHELL=/bin/sh"), Enter])?; + tmux.start_sk(None, &["--tmux", "--bind 'ctrl-a:reload(ls /foo*)'"])?; + tmux.until(|_| Path::new(&outfile).exists())?; + let cmd = get_tmux_cmd(&outfile)?; + assert!(cmd.starts_with("display-popup")); + assert!(cmd.contains("-E")); + assert!(cmd.contains("--bind ctrl-a':reload(ls /foo*)'")); + + Ok(()) +} diff --git a/skim/Cargo.toml b/skim/Cargo.toml index 75b7d383..a37d1e55 100644 --- a/skim/Cargo.toml +++ b/skim/Cargo.toml @@ -21,7 +21,7 @@ name = "sk" path = "src/bin/main.rs" [dependencies] -nix = "0.29.0" +nix = { version = "0.29.0", features = ["fs"] } atty = { version = "0.2.14", optional = true } regex = "1.6.0" lazy_static = "1.4.0" @@ -45,6 +45,7 @@ defer-drop = "1.3.0" indexmap = "2.7.0" rand = "0.8.5" which = "7.0.0" +shell-quote = "0.7.1" [features] default = ["cli"] diff --git a/skim/src/item.rs b/skim/src/item.rs index 7d7eb472..2c1c6901 100644 --- a/skim/src/item.rs +++ b/skim/src/item.rs @@ -146,6 +146,10 @@ impl ItemPool { self.length.load(Ordering::SeqCst) } + pub fn is_empty(&self) -> bool { + self.len() == 0 + } + pub fn num_not_taken(&self) -> usize { self.length.load(Ordering::SeqCst) - self.taken.load(Ordering::SeqCst) } diff --git a/skim/src/tmux.rs b/skim/src/tmux.rs index 7867d111..8e93f4ba 100644 --- a/skim/src/tmux.rs +++ b/skim/src/tmux.rs @@ -1,11 +1,14 @@ use std::{ borrow::Cow, - io::{BufRead as _, BufReader, BufWriter, IsTerminal as _, Write as _}, + env, + fs::File, + io::{BufRead as _, BufReader, IsTerminal as _, Write as _}, process::{Command, Stdio}, sync::Arc, thread, }; +use nix::{sys::stat, unistd::mkfifo}; use rand::{distributions::Alphanumeric, Rng}; use tuikit::key::Key; use which::which; @@ -108,21 +111,26 @@ pub fn run_with(opts: &SkimOptions) -> Option { let mut stdin_reader = BufReader::new(std::io::stdin()); let line_ending = if opts.read0 { b'\0' } else { b'\n' }; + let t_tmp_stdin = temp_dir.join("stdin"); let stdin_handle = if has_piped_input { debug!("Reading stdin and piping to file"); - let stdin_f = std::fs::File::create(tmp_stdin.clone()) - .unwrap_or_else(|e| panic!("Failed to create stdin file {}: {}", tmp_stdin.clone().display(), e)); - let mut stdin_writer = BufWriter::new(stdin_f); - Some(thread::spawn(move || loop { - let mut buf = vec![]; - match stdin_reader.read_until(line_ending, &mut buf) { - Ok(0) => break, - Ok(n) => { - debug!("Read {n} bytes from stdin"); - stdin_writer.write_all(&buf).unwrap(); + mkfifo(&tmp_stdin, stat::Mode::S_IRWXU) + .unwrap_or_else(|e| panic!("Failed to create stdin pipe {}: {}", tmp_stdin.clone().display(), e)); + Some(thread::spawn(move || { + let mut stdin_writer = File::create(&t_tmp_stdin) + .unwrap_or_else(|e| panic!("Failed to open stdin pipe {}: {}", t_tmp_stdin.clone().display(), e)); + + loop { + let mut buf = vec![]; + match stdin_reader.read_until(line_ending, &mut buf) { + Ok(0) => break, + Ok(n) => { + debug!("Read {n} bytes from stdin"); + stdin_writer.write_all(&buf).unwrap(); + } + Err(e) => panic!("Failed to read from stdin: {}", e), } - Err(e) => panic!("Failed to read from stdin: {}", e), } })) } else { @@ -149,7 +157,7 @@ pub fn run_with(opts: &SkimOptions) -> Option { debug!("Found equal tmux arg, skipping"); continue; } - tmux_shell_cmd.push_str(&format!(" {arg}")); + push_quoted_arg(&mut tmux_shell_cmd, &arg); } if has_piped_input { tmux_shell_cmd.push_str(&format!(" <{}", tmp_stdin.display())); @@ -173,7 +181,7 @@ pub fn run_with(opts: &SkimOptions) -> Option { .args(["-y", tmux_opts.y]); for (name, value) in std::env::vars() { - if name.starts_with("SKIM") || name == "PATH" { + if name.starts_with("SKIM") || name == "PATH" || name.starts_with("RUST") { debug!("adding {} = {} to the command's env", name, value); tmux_cmd.args(["-e", &format!("{}={}", name, value)]); } @@ -235,6 +243,23 @@ pub fn run_with(opts: &SkimOptions) -> Option { }; Some(skim_output) } + +fn push_quoted_arg(args_str: &mut String, arg: &str) { + use shell_quote::{Bash, Fish, Quote as _, Sh, Zsh}; + let shell_path = env::var("SHELL").unwrap_or(String::from("/bin/sh")); + let shell = shell_path.rsplit_once('/').unwrap_or(("", "sh")).1; + let quoted_arg: Vec = match shell { + "zsh" => Zsh::quote(arg), + "bash" => Bash::quote(arg), + "fish" => Fish::quote(arg), + _ => Sh::quote(arg), + }; + args_str.push_str(&format!( + " {}", + String::from_utf8(quoted_arg).expect("Failed to parse quoted arg as utf8, this should not happen") + )); +} + #[cfg(test)] mod tests { use super::*;