Some cleanup and refactor

src/app.rs

@@ -219,7 +219,7 @@ pub fn build_app() -> App<'static, 'static> {
         )
 }
 
-#[cfg_attr(rustfmt, rustfmt_skip)]
+#[rustfmt::skip]
 fn usage() -> HashMap<&'static str, Help> {
     let mut h = HashMap::new();
     doc!(h, "hidden"

src/exec/input.rs

@@ -71,90 +71,46 @@ pub fn dirname(path: &str) -> &str {
 }
 
 #[cfg(test)]
-mod tests {
+mod path_tests {
     use super::*;
 
     fn correct(input: &str) -> String {
         input.replace('/', &MAIN_SEPARATOR.to_string())
     }
 
-    #[test]
-    fn path_remove_ext_simple() {
-        assert_eq!(remove_extension("foo.txt"), "foo");
-    }
-
-    #[test]
-    fn path_remove_ext_dir() {
-        assert_eq!(
-            remove_extension(&correct("dir/foo.txt")),
-            correct("dir/foo")
-        );
-    }
-
-    #[test]
-    fn path_hidden() {
-        assert_eq!(remove_extension(".foo"), ".foo")
-    }
-
-    #[test]
-    fn path_remove_ext_utf8() {
-        assert_eq!(remove_extension("💖.txt"), "💖");
-    }
-
-    #[test]
-    fn path_remove_ext_empty() {
-        assert_eq!(remove_extension(""), "");
-    }
-
-    #[test]
-    fn path_basename_simple() {
-        assert_eq!(basename("foo.txt"), "foo.txt");
-    }
-
-    #[test]
-    fn path_basename_no_ext() {
-        assert_eq!(remove_extension(basename("foo.txt")), "foo");
-    }
-
-    #[test]
-    fn path_basename_dir() {
-        assert_eq!(basename(&correct("dir/foo.txt")), "foo.txt");
-    }
-
-    #[test]
-    fn path_basename_empty() {
-        assert_eq!(basename(""), "");
-    }
-
-    #[test]
-    fn path_basename_utf8() {
-        assert_eq!(basename(&correct("💖/foo.txt")), "foo.txt");
-        assert_eq!(basename(&correct("dir/💖.txt")), "💖.txt");
+    macro_rules! func_tests {
+        ($($name:ident: $func:ident for $input:expr => $output:expr)+) => {
+            $(
+                #[test]
+                fn $name() {
+                    assert_eq!($func(&correct($input)), correct($output));
+                }
+            )+
+        }
     }
 
-    #[test]
-    fn path_dirname_simple() {
-        assert_eq!(dirname("foo.txt"), ".");
-    }
+    func_tests! {
+        remove_ext_simple:  remove_extension  for  "foo.txt"      =>  "foo"
+        remove_ext_dir:     remove_extension  for  "dir/foo.txt"  =>  "dir/foo"
+        hidden:             remove_extension  for  ".foo"         =>  ".foo"
+        remove_ext_utf8:    remove_extension  for  "💖.txt"       =>  "💖"
+        remove_ext_empty:   remove_extension  for  ""             =>  ""
 
-    #[test]
-    fn path_dirname_dir() {
-        assert_eq!(dirname(&correct("dir/foo.txt")), "dir");
-    }
+        basename_simple:  basename  for  "foo.txt"      =>  "foo.txt"
+        basename_dir:     basename  for  "dir/foo.txt"  =>  "foo.txt"
+        basename_empty:   basename  for  ""             =>  ""
+        basename_utf8_0:  basename  for  "💖/foo.txt"   =>  "foo.txt"
+        basename_utf8_1:  basename  for  "dir/💖.txt"   =>  "💖.txt"
 
-    #[test]
-    fn path_dirname_utf8() {
-        assert_eq!(dirname(&correct("💖/foo.txt")), "💖");
-        assert_eq!(dirname(&correct("dir/💖.txt")), "dir");
-    }
-
-    #[test]
-    fn path_dirname_empty() {
-        assert_eq!(dirname(""), ".");
+        dirname_simple:  dirname  for  "foo.txt"      =>  "."
+        dirname_dir:     dirname  for  "dir/foo.txt"  =>  "dir"
+        dirname_utf8_0:  dirname  for  "💖/foo.txt"   =>  "💖"
+        dirname_utf8_1:  dirname  for  "dir/💖.txt"   =>  "dir"
+        dirname_empty:   dirname  for  ""             =>  "."
     }
 
     #[test]
-    fn path_dirname_root() {
+    fn dirname_root() {
         #[cfg(windows)]
         assert_eq!(dirname("C:\\"), "C:");
         #[cfg(windows)]

src/internal/filter/size.rs

@@ -24,7 +24,7 @@ const GIBI: u64 = MEBI * 1024;
 const TEBI: u64 = GIBI * 1024;
 
 impl SizeFilter {
-    pub fn from_string<'a>(s: &str) -> Option<Self> {
+    pub fn from_string(s: &str) -> Option<Self> {
         if !SIZE_CAPTURES.is_match(s) {
             return None;
         }
@@ -56,9 +56,9 @@ impl SizeFilter {
     }
 
     pub fn is_within(&self, size: u64) -> bool {
-        match self {
-            &SizeFilter::Max(limit) => size <= limit,
-            &SizeFilter::Min(limit) => size >= limit,
+        match *self {
+            SizeFilter::Max(limit) => size <= limit,
+            SizeFilter::Min(limit) => size >= limit,
         }
     }
 }

src/main.rs

@@ -49,7 +49,7 @@ fn main() {
     // Get one or more root directories to search.
     let mut dir_vec: Vec<_> = match matches
         .values_of("path")
-        .or(matches.values_of("search-path"))
+        .or_else(|| matches.values_of("search-path"))
     {
         Some(paths) => paths
             .map(|path| {

src/output.rs

@@ -19,7 +19,7 @@ use std::sync::Arc;
 use ansi_term;
 
 /// Remove the `./` prefix from a path.
-fn strip_current_dir<'a>(pathbuf: &'a PathBuf) -> &'a Path {
+fn strip_current_dir(pathbuf: &PathBuf) -> &Path {
     let mut iter = pathbuf.components();
     let mut iter_next = iter.clone();
     if iter_next.next() == Some(Component::CurDir) {
@@ -70,15 +70,15 @@ fn print_entry_colorized(
         write!(stdout, "{}", style.paint(component.to_string_lossy()))?;
 
         if wants_to_quit.load(Ordering::Relaxed) {
-            write!(stdout, "\n")?;
+            writeln!(stdout)?;
             process::exit(ExitCode::KilledBySigint.into());
         }
     }
 
     if config.null_separator {
         write!(stdout, "\0")
     } else {
-        writeln!(stdout, "")
+        writeln!(stdout)
     }
 }
 

src/walk.rs

@@ -17,7 +17,7 @@ use std::io;
 use std::path::PathBuf;
 use std::process;
 use std::sync::atomic::{AtomicBool, Ordering};
-use std::sync::mpsc::channel;
+use std::sync::mpsc::{channel, Receiver, Sender};
 use std::sync::{Arc, Mutex};
 use std::thread;
 use std::time;
@@ -53,8 +53,6 @@ pub fn scan(path_vec: &[PathBuf], pattern: Arc<Regex>, config: Arc<FdOptions>) {
         .next()
         .expect("Error: Path vector can not be empty");
     let (tx, rx) = channel();
-    let threads = config.threads;
-    let show_filesystem_errors = config.show_filesystem_errors;
 
     let mut override_builder = OverrideBuilder::new(first_path_buf.as_path());
 
@@ -86,45 +84,65 @@ pub fn scan(path_vec: &[PathBuf], pattern: Arc<Regex>, config: Arc<FdOptions>) {
 
     for ignore_file in &config.ignore_files {
         let result = walker.add_ignore(ignore_file);
-        if let Some(err) = result {
-            match err {
-                ignore::Error::Partial(_) => (),
-                _ => {
-                    print_error!(
-                        "{}",
-                        format!(
-                            "Malformed pattern in custom ignore file '{}': {}.",
-                            ignore_file.to_string_lossy(),
-                            err.description()
-                        )
-                    );
-                }
+        match result {
+            Some(ignore::Error::Partial(_)) => (),
+            Some(err) => {
+                print_error!(
+                    "{}",
+                    format!(
+                        "Malformed pattern in custom ignore file '{}': {}.",
+                        ignore_file.to_string_lossy(),
+                        err.description()
+                    )
+                );
             }
+            None => (),
         }
     }
 
     for path_entry in path_iter {
         walker.add(path_entry.as_path());
     }
 
-    let parallel_walker = walker.threads(threads).build_parallel();
+    let parallel_walker = walker.threads(config.threads).build_parallel();
 
     let wants_to_quit = Arc::new(AtomicBool::new(false));
-    let receiver_wtq = Arc::clone(&wants_to_quit);
-    let sender_wtq = Arc::clone(&wants_to_quit);
     if config.ls_colors.is_some() && config.command.is_none() {
-        let wq = Arc::clone(&receiver_wtq);
+        let wq = Arc::clone(&wants_to_quit);
         ctrlc::set_handler(move || {
             wq.store(true, Ordering::Relaxed);
         })
         .unwrap();
     }
 
     // Spawn the thread that receives all results through the channel.
-    let rx_config = Arc::clone(&config);
-    let receiver_thread = thread::spawn(move || {
+    let receiver_thread = spawn_receiver(&config, &wants_to_quit, rx);
+
+    // Spawn the sender threads.
+    spawn_senders(&config, &wants_to_quit, pattern, parallel_walker, tx);
+
+    // Wait for the receiver thread to print out all results.
+    receiver_thread.join().unwrap();
+
+    if wants_to_quit.load(Ordering::Relaxed) {
+        process::exit(ExitCode::KilledBySigint.into());
+    }
+}
+
+fn spawn_receiver(
+    config: &Arc<FdOptions>,
+    wants_to_quit: &Arc<AtomicBool>,
+    rx: Receiver<WorkerResult>,
+) -> thread::JoinHandle<()> {
+    let config = Arc::clone(config);
+    let wants_to_quit = Arc::clone(wants_to_quit);
+
+    let show_filesystem_errors = config.show_filesystem_errors;
+    let threads = config.threads;
+
+    thread::spawn(move || {
         // This will be set to `Some` if the `--exec` argument was supplied.
-        if let Some(ref cmd) = rx_config.command {
+        if let Some(ref cmd) = config.command {
             if cmd.in_batch_mode() {
                 exec::batch(rx, cmd, show_filesystem_errors);
             } else {
@@ -167,7 +185,7 @@ pub fn scan(path_vec: &[PathBuf], pattern: Arc<Regex>, config: Arc<FdOptions>) {
             let mut mode = ReceiverMode::Buffering;
 
             // Maximum time to wait before we start streaming to the console.
-            let max_buffer_time = rx_config
+            let max_buffer_time = config
                 .max_buffer_time
                 .unwrap_or_else(|| time::Duration::from_millis(100));
 
@@ -190,8 +208,8 @@ pub fn scan(path_vec: &[PathBuf], pattern: Arc<Regex>, config: Arc<FdOptions>) {
                                         output::print_entry(
                                             &mut stdout,
                                             v,
-                                            &rx_config,
-                                            &receiver_wtq,
+                                            &config,
+                                            &wants_to_quit,
                                         );
                                     }
                                     buffer.clear();
@@ -201,7 +219,7 @@ pub fn scan(path_vec: &[PathBuf], pattern: Arc<Regex>, config: Arc<FdOptions>) {
                                 }
                             }
                             ReceiverMode::Streaming => {
-                                output::print_entry(&mut stdout, &value, &rx_config, &receiver_wtq);
+                                output::print_entry(&mut stdout, &value, &config, &wants_to_quit);
                             }
                         }
                     }
@@ -218,18 +236,25 @@ pub fn scan(path_vec: &[PathBuf], pattern: Arc<Regex>, config: Arc<FdOptions>) {
             if !buffer.is_empty() {
                 buffer.sort();
                 for value in buffer {
-                    output::print_entry(&mut stdout, &value, &rx_config, &receiver_wtq);
+                    output::print_entry(&mut stdout, &value, &config, &wants_to_quit);
                 }
             }
         }
-    });
+    })
+}
 
-    // Spawn the sender threads.
+fn spawn_senders(
+    config: &Arc<FdOptions>,
+    wants_to_quit: &Arc<AtomicBool>,
+    pattern: Arc<Regex>,
+    parallel_walker: ignore::WalkParallel,
+    tx: Sender<WorkerResult>,
+) {
     parallel_walker.run(|| {
-        let config = Arc::clone(&config);
+        let config = Arc::clone(config);
         let pattern = Arc::clone(&pattern);
         let tx_thread = tx.clone();
-        let wants_to_quit = Arc::clone(&sender_wtq);
+        let wants_to_quit = Arc::clone(wants_to_quit);
 
         Box::new(move |entry_o| {
             if wants_to_quit.load(Ordering::Relaxed) {
@@ -279,7 +304,7 @@ pub fn scan(path_vec: &[PathBuf], pattern: Arc<Regex>, config: Arc<FdOptions>) {
 
             // Filter out unwanted extensions.
             if let Some(ref exts_regex) = config.extensions {
-                if let Some(path_str) = entry_path.file_name().map_or(None, |s| s.to_str()) {
+                if let Some(path_str) = entry_path.file_name().and_then(|s| s.to_str()) {
                     if !exts_regex.is_match(path_str) {
                         return ignore::WalkState::Continue;
                     }
@@ -289,7 +314,7 @@ pub fn scan(path_vec: &[PathBuf], pattern: Arc<Regex>, config: Arc<FdOptions>) {
             }
 
             // Filter out unwanted sizes if it is a file and we have been given size constraints.
-            if config.size_constraints.len() > 0 {
+            if !config.size_constraints.is_empty() {
                 if entry_path.is_file() {
                     if let Ok(metadata) = entry_path.metadata() {
                         let file_size = metadata.len();
@@ -349,15 +374,4 @@ pub fn scan(path_vec: &[PathBuf], pattern: Arc<Regex>, config: Arc<FdOptions>) {
             ignore::WalkState::Continue
         })
     });
-
-    // Drop the initial sender. If we don't do this, the receiver will block even
-    // if all threads have finished, since there is still one sender around.
-    drop(tx);
-
-    // Wait for the receiver thread to print out all results.
-    receiver_thread.join().unwrap();
-
-    if wants_to_quit.load(Ordering::Relaxed) {
-        process::exit(ExitCode::KilledBySigint.into());
-    }
 }