fix: fix --tmux quoting (#643)

* fix: fix --tmux quoting * fix e2e * use fifo for input * fix macos e2e --------- Co-authored-by: LoricAndre <loric.andre@pm.me>
skim-rs · Dec 3, 2024 · 1abf545 · 1abf545
1 parent bdaac9c
commit 1abf545
Show file tree

Hide file tree

Showing 5 changed files with 125 additions and 16 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/e2e/tests/tmux.rs b/e2e/tests/tmux.rs
@@ -17,7 +17,7 @@ fn setup_tmux_mock(tmux: &TmuxController) -> Result<String> {
     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
@@ -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
@@ -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
@@ -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<SkimOutput> {
     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<SkimOutput> {
             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<SkimOutput> {
         .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<SkimOutput> {
     };
     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<u8> = 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::*;