Repository: sharkdp/fd
Pull request #360 (2018-11-11 17:10:51)

Add support for batch execution of command
This aims at implementing #274 

Note: for this first version, arg_max is not taken into account.

kimsnj: Thanks for the review!

Looking at:
```
fd -X "echo {}"
```
I only read from the `rx` iterator chain for arguments other than first one (supposed to be the executable).
So in this case, the receiver is not read at all, and thus the thread ends fast, drops `rx`… And the transmitters panics when trying to send!

I see a few options to handle this case:
- Just apply the same generation on the first argument. This would launch `echo a echo b echo c…`
- In case of a single argument in input, split by white-space before parsing the command.
- Reject entries that do not have a ArgumentTemplate::Text as first arg.

Any opinions?

I would be tempted by option 2, but not sure if there are some weird cases to consider.
sharkdp: > Any opinions?
> 
> I would be tempted by option 2, but not sure if there are some weird cases to consider.

I would just go with option three. I think it doesn't make sense to allow placeholders in the first argument for `--exec-once` (for `--exec`, you could use something like `fd --type executable --exec "{}"` to run all executables in a directory).

Patch:
diff --git a/src/app.rs b/src/app.rs
index 4b87d56..95edcad 100644
--- a/src/app.rs
+++ b/src/app.rs
@@ -121,6 +121,16 @@ pub fn build_app() -> App<'static, 'static> {
                 .value_terminator(";")
                 .value_name("cmd"),
         )
+        .arg(
+            arg("exec-batch")
+                .long("exec-batch")
+                .short("X")
+                .min_values(1)
+                .allow_hyphen_values(true)
+                .value_terminator(";")
+                .value_name("cmd")
+                .conflicts_with("exec"),
+        )
         .arg(
             arg("exclude")
                 .long("exclude")
@@ -277,6 +287,18 @@ fn usage() -> HashMap<&'static str, Help> {
              '{//}': parent directory\n  \
              '{.}':  path without file extension\n  \
              '{/.}': basename without file extension");
+    doc!(h, "exec-batch"
+        , "Execute a command with all search results at once"
+        , "Execute a command with all search results at once.\n\
+           All arguments following --exec-batch are taken to be arguments to the command until the \
+           argument ';' is encountered.\n\
+           A single occurence of the following placeholders is authorized and substituted by the paths derived from the \
+           search results before the command is executed:\n  \
+             '{}':   path\n  \
+             '{/}':  basename\n  \
+             '{//}': parent directory\n  \
+             '{.}':  path without file extension\n  \
+             '{/.}': basename without file extension");
     doc!(h, "exclude"
         , "Exclude entries that match the given glob pattern"
         , "Exclude files/directories that match the given glob pattern. This overrides any \
diff --git a/src/exec/command.rs b/src/exec/command.rs
index 1d4f900..f02b5f4 100644
--- a/src/exec/command.rs
+++ b/src/exec/command.rs
@@ -9,10 +9,10 @@
 use std::io;
 use std::io::Write;
 use std::process::Command;
-use std::sync::{Arc, Mutex};
+use std::sync::Mutex;
 
 /// Executes a command.
-pub fn execute_command(mut cmd: Command, out_perm: Arc<Mutex<()>>) {
+pub fn execute_command(mut cmd: Command, out_perm: &Mutex<()>) {
     // Spawn the supplied command.
     let output = cmd.output();
 
diff --git a/src/exec/job.rs b/src/exec/job.rs
index 51361d3..b961020 100644
--- a/src/exec/job.rs
+++ b/src/exec/job.rs
@@ -44,3 +44,16 @@ pub fn job(
         cmd.generate_and_execute(&value, Arc::clone(&out_perm));
     }
 }
+
+pub fn batch(rx: Receiver<WorkerResult>, cmd: &CommandTemplate, show_filesystem_errors: bool) {
+    let paths = rx.iter().filter_map(|value| match value {
+        WorkerResult::Entry(val) => Some(val),
+        WorkerResult::Error(err) => {
+            if show_filesystem_errors {
+                print_error!("{}", err);
+            }
+            None
+        }
+    });
+    cmd.generate_and_execute_batch(paths);
+}
diff --git a/src/exec/mod.rs b/src/exec/mod.rs
index 0cac764..6b58661 100644
--- a/src/exec/mod.rs
+++ b/src/exec/mod.rs
@@ -13,17 +13,26 @@ mod job;
 mod token;
 
 use std::borrow::Cow;
-use std::path::Path;
-use std::process::Command;
+use std::path::{Path, PathBuf};
+use std::process::{Command, Stdio};
 use std::sync::{Arc, Mutex};
 
 use regex::Regex;
 
 use self::command::execute_command;
 use self::input::{basename, dirname, remove_extension};
-pub use self::job::job;
+pub use self::job::{batch, job};
 use self::token::Token;
 
+/// Execution mode of the command
+#[derive(Debug, Clone, Copy, PartialEq)]
+pub enum ExecutionMode {
+    /// Command is executed for each search result
+    OneByOne,
+    /// Command is run for a batch of results at once
+    Batch,
+}
+
 /// Represents a template that is utilized to generate command strings.
 ///
 /// The template is meant to be coupled with an input in order to generate a command. The
@@ -31,10 +40,34 @@ use self::token::Token;
 #[derive(Debug, Clone, PartialEq)]
 pub struct CommandTemplate {
     args: Vec<ArgumentTemplate>,
+    mode: ExecutionMode,
 }
 
 impl CommandTemplate {
     pub fn new<I, S>(input: I) -> CommandTemplate
+    where
+        I: IntoIterator<Item = S>,
+        S: AsRef<str>,
+    {
+        Self::build(input, ExecutionMode::OneByOne)
+    }
+
+    pub fn new_batch<I, S>(input: I) -> Result<CommandTemplate, &'static str>
+    where
+        I: IntoIterator<Item = S>,
+        S: AsRef<str>,
+    {
+        let cmd = Self::build(input, ExecutionMode::Batch);
+        if cmd.number_of_tokens() > 1 {
+            return Err("Only one placeholder allowed for batch commands");
+        }
+        if cmd.args[0].has_tokens() {
+            return Err("First argument of exec-batch is expected to be a fixed executable");
+        }
+        Ok(cmd)
+    }
+
+    fn build<I, S>(input: I, mode: ExecutionMode) -> CommandTemplate
     where
         I: IntoIterator<Item = S>,
         S: AsRef<str>,
@@ -91,7 +124,19 @@ impl CommandTemplate {
             args.push(ArgumentTemplate::Tokens(vec![Token::Placeholder]));
         }
 
-        CommandTemplate { args }
+        CommandTemplate { args, mode }
+    }
+
+    fn number_of_tokens(&self) -> usize {
+        self.args.iter().filter(|arg| arg.has_tokens()).count()
+    }
+
+    fn prepare_path(input: &Path) -> String {
+        input
+            .strip_prefix(".")
+            .unwrap_or(input)
+            .to_string_lossy()
+            .into_owned()
     }
 
     /// Generates and executes a command.
@@ -99,18 +144,48 @@ impl CommandTemplate {
     /// Using the internal `args` field, and a supplied `input` variable, a `Command` will be
     /// build. Once all arguments have been processed, the command is executed.
     pub fn generate_and_execute(&self, input: &Path, out_perm: Arc<Mutex<()>>) {
-        let input = input
-            .strip_prefix(".")
-            .unwrap_or(input)
-            .to_string_lossy()
-            .into_owned();
+        let input = Self::prepare_path(input);
 
         let mut cmd = Command::new(self.args[0].generate(&input).as_ref());
         for arg in &self.args[1..] {
             cmd.arg(arg.generate(&input).as_ref());
         }
 
-        execute_command(cmd, out_perm)
+        execute_command(cmd, &out_perm)
+    }
+
+    pub fn in_batch_mode(&self) -> bool {
+        self.mode == ExecutionMode::Batch
+    }
+
+    pub fn generate_and_execute_batch<I>(&self, paths: I)
+    where
+        I: Iterator<Item = PathBuf>,
+    {
+        let mut cmd = Command::new(self.args[0].generate("").as_ref());
+        cmd.stdin(Stdio::inherit());
+        cmd.stdout(Stdio::inherit());
+        cmd.stderr(Stdio::inherit());
+
+        let mut paths = paths.map(|p| Self::prepare_path(&p));
+        let mut has_path = false;
+
+        for arg in &self.args[1..] {
+            if arg.has_tokens() {
+                // A single `Tokens` is expected
+                // So we can directy consume the iterator once and for all
+                for path in &mut paths {
+                    cmd.arg(arg.generate(&path).as_ref());
+                    has_path = true;
+                }
+            } else {
+                cmd.arg(arg.generate("").as_ref());
+            }
+        }
+
+        if has_path {
+            execute_command(cmd, &Mutex::new(()));
+        }
     }
 }
 
@@ -125,6 +200,13 @@ enum ArgumentTemplate {
 }
 
 impl ArgumentTemplate {
+    pub fn has_tokens(&self) -> bool {
+        match self {
+            ArgumentTemplate::Tokens(_) => true,
+            _ => false,
+        }
+    }
+
     pub fn generate<'a>(&'a self, path: &str) -> Cow<'a, str> {
         use self::Token::*;
 
@@ -162,6 +244,7 @@ mod tests {
                     ArgumentTemplate::Text("${SHELL}:".into()),
                     ArgumentTemplate::Tokens(vec![Token::Placeholder]),
                 ],
+                mode: ExecutionMode::OneByOne,
             }
         );
     }
@@ -175,6 +258,7 @@ mod tests {
                     ArgumentTemplate::Text("echo".into()),
                     ArgumentTemplate::Tokens(vec![Token::NoExt]),
                 ],
+                mode: ExecutionMode::OneByOne,
             }
         );
     }
@@ -188,6 +272,7 @@ mod tests {
                     ArgumentTemplate::Text("echo".into()),
                     ArgumentTemplate::Tokens(vec![Token::Basename]),
                 ],
+                mode: ExecutionMode::OneByOne,
             }
         );
     }
@@ -201,6 +286,7 @@ mod tests {
                     ArgumentTemplate::Text("echo".into()),
                     ArgumentTemplate::Tokens(vec![Token::Parent]),
                 ],
+                mode: ExecutionMode::OneByOne,
             }
         );
     }
@@ -214,6 +300,7 @@ mod tests {
                     ArgumentTemplate::Text("echo".into()),
                     ArgumentTemplate::Tokens(vec![Token::BasenameNoExt]),
                 ],
+                mode: ExecutionMode::OneByOne,
             }
         );
     }
@@ -231,7 +318,27 @@ mod tests {
                         Token::Text(".ext".into())
                     ]),
                 ],
+                mode: ExecutionMode::OneByOne,
             }
         );
     }
+
+    #[test]
+    fn tokens_single_batch() {
+        assert_eq!(
+            CommandTemplate::new_batch(&["echo", "{.}"]).unwrap(),
+            CommandTemplate {
+                args: vec![
+                    ArgumentTemplate::Text("echo".into()),
+                    ArgumentTemplate::Tokens(vec![Token::NoExt]),
+                ],
+                mode: ExecutionMode::Batch,
+            }
+        );
+    }
+
+    #[test]
+    fn tokens_multiple_batch() {
+        assert!(CommandTemplate::new_batch(&["echo", "{.}", "{}"]).is_err());
+    }
 }
diff --git a/src/main.rs b/src/main.rs
index e792565..5c87854 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -142,7 +142,16 @@ fn main() {
         None
     };
 
-    let command = matches.values_of("exec").map(CommandTemplate::new);
+    let command = matches
+        .values_of("exec")
+        .map(CommandTemplate::new)
+        .or_else(|| {
+            matches.values_of("exec-batch").map(|m| {
+                CommandTemplate::new_batch(m).unwrap_or_else(|e| {
+                    print_error_and_exit!("{}", e);
+                })
+            })
+        });
 
     let size_limits: Vec<SizeFilter> = matches
         .values_of("size")
diff --git a/src/walk.rs b/src/walk.rs
index b1684cf..7943e29 100644
--- a/src/walk.rs
+++ b/src/walk.rs
@@ -126,34 +126,38 @@ pub fn scan(path_vec: &[PathBuf], pattern: Arc<Regex>, config: Arc<FdOptions>) {
     let receiver_thread = thread::spawn(move || {
         // This will be set to `Some` if the `--exec` argument was supplied.
         if let Some(ref cmd) = rx_config.command {
-            let shared_rx = Arc::new(Mutex::new(rx));
+            if cmd.in_batch_mode() {
+                exec::batch(rx, cmd, show_filesystem_errors);
+            } else {
+                let shared_rx = Arc::new(Mutex::new(rx));
 
-            let out_perm = Arc::new(Mutex::new(()));
+                let out_perm = Arc::new(Mutex::new(()));
 
-            // TODO: the following line is a workaround to replace the `unsafe` block that was
-            // previously used here to avoid the (unnecessary?) cloning of the command. The
-            // `unsafe` block caused problems on some platforms (SIGILL instructions on Linux) and
-            // therefore had to be removed.
-            let cmd = Arc::new(cmd.clone());
+                // TODO: the following line is a workaround to replace the `unsafe` block that was
+                // previously used here to avoid the (unnecessary?) cloning of the command. The
+                // `unsafe` block caused problems on some platforms (SIGILL instructions on Linux) and
+                // therefore had to be removed.
+                let cmd = Arc::new(cmd.clone());
 
-            // Each spawned job will store it's thread handle in here.
-            let mut handles = Vec::with_capacity(threads);
-            for _ in 0..threads {
-                let rx = Arc::clone(&shared_rx);
-                let cmd = Arc::clone(&cmd);
-                let out_perm = Arc::clone(&out_perm);
+                // Each spawned job will store it's thread handle in here.
+                let mut handles = Vec::with_capacity(threads);
+                for _ in 0..threads {
+                    let rx = Arc::clone(&shared_rx);
+                    let cmd = Arc::clone(&cmd);
+                    let out_perm = Arc::clone(&out_perm);
 
-                // Spawn a job thread that will listen for and execute inputs.
-                let handle =
-                    thread::spawn(move || exec::job(rx, cmd, out_perm, show_filesystem_errors));
+                    // Spawn a job thread that will listen for and execute inputs.
+                    let handle =
+                        thread::spawn(move || exec::job(rx, cmd, out_perm, show_filesystem_errors));
 
-                // Push the handle of the spawned thread into the vector for later joining.
-                handles.push(handle);
-            }
+                    // Push the handle of the spawned thread into the vector for later joining.
+                    handles.push(handle);
+                }
 
-            // Wait for all threads to exit before exiting the program.
-            for h in handles {
-                h.join().unwrap();
+                // Wait for all threads to exit before exiting the program.
+                for h in handles {
+                    h.join().unwrap();
+                }
             }
         } else {
             let start = time::Instant::now();


Test patch:
diff --git a/tests/testenv/mod.rs b/tests/testenv/mod.rs
index 1d535f8..1a6f9d9 100644
--- a/tests/testenv/mod.rs
+++ b/tests/testenv/mod.rs
@@ -32,6 +32,9 @@ pub struct TestEnv {
 
     /// Path to the *fd* executable.
     fd_exe: PathBuf,
+
+    /// Normalize each line by sorting the whitespace-separated words
+    normalize_line: bool,
 }
 
 /// Create the working directory and the test files.
@@ -121,19 +124,24 @@ fn format_output_error(args: &[&str], expected: &str, actual: &str) -> String {
 }
 
 /// Normalize the output for comparison.
-fn normalize_output(s: &str, trim_left: bool) -> String {
+fn normalize_output(s: &str, trim_left: bool, normalize_line: bool) -> String {
     // Split into lines and normalize separators.
     let mut lines = s
         .replace('\0', "NULL\n")
         .lines()
         .map(|line| {
             let line = if trim_left { line.trim_left() } else { line };
-            line.replace('/', &std::path::MAIN_SEPARATOR.to_string())
+            let line = line.replace('/', &std::path::MAIN_SEPARATOR.to_string());
+            if normalize_line {
+                let mut words: Vec<_> = line.split_whitespace().collect();
+                words.sort();
+                return words.join(" ");
+            }
+            line
         })
         .collect::<Vec<_>>();
 
-    lines.sort_by_key(|s| s.clone());
-
+    lines.sort();
     lines.join("\n")
 }
 
@@ -143,8 +151,17 @@ impl TestEnv {
         let fd_exe = find_fd_exe();
 
         TestEnv {
-            temp_dir: temp_dir,
-            fd_exe: fd_exe,
+            temp_dir,
+            fd_exe,
+            normalize_line: false,
+        }
+    }
+
+    pub fn normalize_line(self, normalize: bool) -> TestEnv {
+        TestEnv {
+            temp_dir: self.temp_dir,
+            fd_exe: self.fd_exe,
+            normalize_line: normalize,
         }
     }
 
@@ -186,12 +203,44 @@ impl TestEnv {
         }
 
         // Normalize both expected and actual output.
-        let expected = normalize_output(expected, true);
-        let actual = normalize_output(&String::from_utf8_lossy(&output.stdout), false);
+        let expected = normalize_output(expected, true, self.normalize_line);
+        let actual = normalize_output(
+            &String::from_utf8_lossy(&output.stdout),
+            false,
+            self.normalize_line,
+        );
 
         // Compare actual output to expected output.
         if expected != actual {
             panic!(format_output_error(args, &expected, &actual));
         }
     }
+
+    /// Assert that calling *fd* with the specified arguments produces the expected error.
+    pub fn assert_error(&self, args: &[&str], expected: &str) {
+        self.assert_error_subdirectory(".", args, expected)
+    }
+
+    /// Assert that calling *fd* in the specified path under the root working directory,
+    /// and with the specified arguments produces an error with the expected message.
+    fn assert_error_subdirectory<P: AsRef<Path>>(&self, path: P, args: &[&str], expected: &str) {
+        // Setup *fd* command.
+        let mut cmd = process::Command::new(&self.fd_exe);
+        cmd.current_dir(self.temp_dir.path().join(path));
+        cmd.args(args);
+
+        // Run *fd*.
+        let output = cmd.output().expect("fd output");
+
+        // Check for exit status.
+        if output.status.success() {
+            panic!("error '{}' did not occur.", expected);
+        }
+
+        // Compare actual output to expected output.
+        let actual = String::from_utf8_lossy(&output.stderr);
+        if !actual.starts_with(expected) {
+            panic!(format_output_error(args, &expected, &actual));
+        }
+    }
 }
diff --git a/tests/tests.rs b/tests/tests.rs
index f9ac024..8659043 100644
--- a/tests/tests.rs
+++ b/tests/tests.rs
@@ -987,6 +987,65 @@ fn assert_exec_output(exec_style: &str) {
     }
 }
 
+#[test]
+fn test_exec_batch() {
+    assert_exec_batch_output("--exec-batch");
+}
+
+#[test]
+fn test_exec_batch_short_arg() {
+    assert_exec_batch_output("-X");
+}
+
+#[cfg(test)]
+fn assert_exec_batch_output(exec_style: &str) {
+    let (te, abs_path) = get_test_env_with_abs_path(DEFAULT_DIRS, DEFAULT_FILES);
+    let te = te.normalize_line(true);
+
+    // TODO Test for windows
+    if !cfg!(windows) {
+        te.assert_output(
+            &["--absolute-path", "foo", exec_style, "echo"],
+            &format!(
+                "{abs_path}/a.foo {abs_path}/one/b.foo {abs_path}/one/two/C.Foo2 {abs_path}/one/two/c.foo {abs_path}/one/two/three/d.foo {abs_path}/one/two/three/directory_foo",
+                abs_path = &abs_path
+            ),
+        );
+
+        te.assert_output(
+            &["foo", exec_style, "echo", "{}"],
+            "a.foo one/b.foo one/two/C.Foo2 one/two/c.foo one/two/three/d.foo one/two/three/directory_foo",
+        );
+
+        te.assert_output(
+            &["foo", exec_style, "echo", "{/}"],
+            "a.foo b.foo C.Foo2 c.foo d.foo directory_foo",
+        );
+
+        te.assert_output(&["no_match", exec_style, "echo", "Matched: ", "{/}"], "");
+
+        te.assert_error(
+            &["foo", exec_style, "echo", "{}", "{}"],
+            "[fd error]: Only one placeholder allowed for batch commands",
+        );
+
+        te.assert_error(
+            &["foo", exec_style, "echo", "{/}", ";", "-x", "echo"],
+            "error: The argument '--exec <cmd>' cannot be used with '--exec-batch <cmd>'",
+        );
+
+        te.assert_error(
+            &["foo", exec_style],
+            "error: The argument '--exec-batch <cmd>' requires a value but none was supplied",
+        );
+
+        te.assert_error(
+            &["foo", exec_style, "echo {}"],
+            "[fd error]: First argument of exec-batch is expected to be a fixed executable",
+        );
+    }
+}
+
 /// Literal search (--fixed-strings)
 #[test]
 fn test_fixed_strings() {