Allow -exec to work as --exec (#226)

* Replace usage of `-exec` by `--exec`, except when it is used as a value that is given to `--exec/-exec/-x`.
* Limit usage of --exec option to single instance.

src/app.rs

@@ -90,7 +90,6 @@ pub fn build_app() -> App<'static, 'static> {
             arg("exec")
                 .long("exec")
                 .short("x")
-                .multiple(true)
                 .min_values(1)
                 .allow_hyphen_values(true)
                 .value_terminator(";")

src/internal.rs

@@ -7,6 +7,7 @@
 // according to those terms.
 
 use std::collections::HashSet;
+use std::ffi::OsString;
 use std::process;
 use std::time;
 use std::io::Write;
@@ -109,3 +110,119 @@ pub const EXITCODE_ERROR: i32 = 1;
 
 /// Exit code representing that the process was killed by SIGINT
 pub const EXITCODE_SIGINT: i32 = 130;
+
+/// Traverse args_os, looking for -exec and replacing it with --exec.
+///
+/// # Returns
+///
+/// * The args, with substitution if required
+pub fn transform_args_with_exec<I>(original: I) -> Vec<OsString>
+where
+    I: Iterator<Item = OsString>,
+{
+    let mut in_exec_opt = false;
+    let target = OsString::from("-exec");
+    let long_start = OsString::from("--exec");
+    let short_start = OsString::from("-x");
+    let exec_end = OsString::from(";");
+
+    original.fold(vec![], |mut args, curr| {
+        if in_exec_opt {
+            if curr == exec_end {
+                in_exec_opt = false;
+            }
+            args.push(curr);
+            return args;
+        }
+
+        if curr == target || curr == long_start || curr == short_start {
+            args.push(if curr == target {
+                OsString::from("--exec")
+            } else {
+                curr
+            });
+            in_exec_opt = true;
+        } else {
+            args.push(curr);
+        }
+        args
+    })
+}
+
+#[cfg(test)]
+fn oss(v: &str) -> OsString {
+    OsString::from(v)
+}
+
+/// Ensure that -exec gets transformed into --exec
+#[test]
+fn normal_exec_substitution() {
+    let original = vec![oss("fd"), oss("foo"), oss("-exec"), oss("cmd")];
+    let expected = vec![oss("fd"), oss("foo"), oss("--exec"), oss("cmd")];
+
+    let actual = transform_args_with_exec(original.into_iter());
+    assert_eq!(expected, actual);
+}
+
+/// Ensure that --exec is not touched
+#[test]
+fn passthru_of_original_exec() {
+    let original = vec![oss("fd"), oss("foo"), oss("--exec"), oss("cmd")];
+    let expected = vec![oss("fd"), oss("foo"), oss("--exec"), oss("cmd")];
+
+    let actual = transform_args_with_exec(original.into_iter());
+    assert_eq!(expected, actual);
+}
+
+#[test]
+fn temp_check_that_exec_context_observed() {
+    let original = vec![
+        oss("fd"),
+        oss("foo"),
+        oss("-exec"),
+        oss("cmd"),
+        oss("-exec"),
+        oss("ls"),
+        oss(";"),
+        oss("-exec"),
+        oss("rm"),
+        oss(";"),
+        oss("--exec"),
+        oss("find"),
+        oss("-exec"),
+        oss("rm"),
+        oss(";"),
+        oss("-x"),
+        oss("foo"),
+        oss("-exec"),
+        oss("something"),
+        oss(";"),
+        oss("-exec"),
+    ];
+    let expected = vec![
+        oss("fd"),
+        oss("foo"),
+        oss("--exec"),
+        oss("cmd"),
+        oss("-exec"),
+        oss("ls"),
+        oss(";"),
+        oss("--exec"),
+        oss("rm"),
+        oss(";"),
+        oss("--exec"),
+        oss("find"),
+        oss("-exec"),
+        oss("rm"),
+        oss(";"),
+        oss("-x"),
+        oss("foo"),
+        oss("-exec"),
+        oss("something"),
+        oss(";"),
+        oss("--exec"),
+    ];
+
+    let actual = transform_args_with_exec(original.into_iter());
+    assert_eq!(expected, actual);
+}

src/main.rs

@@ -40,12 +40,13 @@ use atty::Stream;
 use regex::RegexBuilder;
 
 use exec::CommandTemplate;
-use internal::{error, pattern_has_uppercase_char, FdOptions};
+use internal::{error, pattern_has_uppercase_char, transform_args_with_exec, FdOptions};
 use lscolors::LsColors;
 use walk::FileType;
 
 fn main() {
-    let matches = app::build_app().get_matches();
+    let checked_args = transform_args_with_exec(env::args_os());
+    let matches = app::build_app().get_matches_from(checked_args);
 
     // Get the search pattern
     let pattern = matches.value_of("pattern").unwrap_or("");

tests/tests.rs

@@ -43,6 +43,13 @@ fn get_absolute_root_path(env: &TestEnv) -> String {
     path
 }
 
+#[cfg(test)]
+fn get_test_env_with_abs_path(dirs: &[&'static str], files: &[&'static str]) -> (TestEnv, String) {
+    let env = TestEnv::new(dirs, files);
+    let root_path = get_absolute_root_path(&env);
+    (env, root_path)
+}
+
 /// Simple tests
 #[test]
 fn test_simple() {
@@ -406,9 +413,7 @@ fn test_max_depth() {
 /// Absolute paths (--absolute-path)
 #[test]
 fn test_absolute_path() {
-    let te = TestEnv::new(DEFAULT_DIRS, DEFAULT_FILES);
-
-    let abs_path = get_absolute_root_path(&te);
+    let (te, abs_path) = get_test_env_with_abs_path(DEFAULT_DIRS, DEFAULT_FILES);
 
     te.assert_output(
         &["--absolute-path"],
@@ -530,9 +535,7 @@ fn test_extension() {
 /// Symlinks misc
 #[test]
 fn test_symlink() {
-    let te = TestEnv::new(DEFAULT_DIRS, DEFAULT_FILES);
-
-    let abs_path = get_absolute_root_path(&te);
+    let (te, abs_path) = get_test_env_with_abs_path(DEFAULT_DIRS, DEFAULT_FILES);
 
     // From: http://pubs.opengroup.org/onlinepubs/9699919799/functions/getcwd.html
     // The getcwd() function shall place an absolute pathname of the current working directory in
@@ -680,14 +683,28 @@ fn test_excludes() {
 /// Shell script execution (--exec)
 #[test]
 fn test_exec() {
-    let te = TestEnv::new(DEFAULT_DIRS, DEFAULT_FILES);
+    assert_exec_output("--exec");
+}
+
+/// Shell script execution using -exec
+#[test]
+fn test_exec_substitution() {
+    assert_exec_output("-exec");
+}
 
-    let abs_path = get_absolute_root_path(&te);
+// Shell script execution using -x
+#[test]
+fn test_exec_short_arg() {
+    assert_exec_output("-x");
+}
 
+#[cfg(test)]
+fn assert_exec_output(exec_style: &str) {
+    let (te, abs_path) = get_test_env_with_abs_path(DEFAULT_DIRS, DEFAULT_FILES);
     // TODO Windows tests: D:file.txt \file.txt \\server\share\file.txt ...
     if !cfg!(windows) {
         te.assert_output(
-            &["--absolute-path", "foo", "--exec", "echo"],
+            &["--absolute-path", "foo", exec_style, "echo"],
             &format!(
                 "{abs_path}/a.foo
                 {abs_path}/one/b.foo
@@ -700,7 +717,7 @@ fn test_exec() {
         );
 
         te.assert_output(
-            &["foo", "--exec", "echo", "{}"],
+            &["foo", exec_style, "echo", "{}"],
             "a.foo
             one/b.foo
             one/two/C.Foo2
@@ -710,7 +727,7 @@ fn test_exec() {
         );
 
         te.assert_output(
-            &["foo", "--exec", "echo", "{.}"],
+            &["foo", exec_style, "echo", "{.}"],
             "a
             one/b
             one/two/C
@@ -720,7 +737,7 @@ fn test_exec() {
         );
 
         te.assert_output(
-            &["foo", "--exec", "echo", "{/}"],
+            &["foo", exec_style, "echo", "{/}"],
             "a.foo
             b.foo
             C.Foo2
@@ -730,7 +747,7 @@ fn test_exec() {
         );
 
         te.assert_output(
-            &["foo", "--exec", "echo", "{/.}"],
+            &["foo", exec_style, "echo", "{/.}"],
             "a
             b
             C
@@ -740,7 +757,7 @@ fn test_exec() {
         );
 
         te.assert_output(
-            &["foo", "--exec", "echo", "{//}"],
+            &["foo", exec_style, "echo", "{//}"],
             ".
             one
             one/two
@@ -749,6 +766,6 @@ fn test_exec() {
             one/two/three",
         );
 
-        te.assert_output(&["e1", "--exec", "printf", "%s.%s\n"], "e1 e2.");
+        te.assert_output(&["e1", exec_style, "printf", "%s.%s\n"], "e1 e2.");
     }
 }