From bce95274e34f88ca5a861aac1d9e2d407f864fcf Mon Sep 17 00:00:00 2001 From: sharkdp Date: Fri, 3 Apr 2020 20:55:14 +0200 Subject: [PATCH] Proper error handling in main.rs --- Cargo.lock | 7 ++ Cargo.toml | 1 + src/exec/mod.rs | 9 ++- src/main.rs | 187 +++++++++++++++++++++++++----------------------- 4 files changed, 110 insertions(+), 94 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 976207d..0ce5b18 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -24,6 +24,11 @@ dependencies = [ "winapi 0.3.8 (registry+https://github.com/rust-lang/crates.io-index)", ] +[[package]] +name = "anyhow" +version = "1.0.28" +source = "registry+https://github.com/rust-lang/crates.io-index" + [[package]] name = "atty" version = "0.2.14" @@ -115,6 +120,7 @@ name = "fd-find" version = "7.5.0" dependencies = [ "ansi_term 0.12.1 (registry+https://github.com/rust-lang/crates.io-index)", + "anyhow 1.0.28 (registry+https://github.com/rust-lang/crates.io-index)", "atty 0.2.14 (registry+https://github.com/rust-lang/crates.io-index)", "clap 2.33.0 (registry+https://github.com/rust-lang/crates.io-index)", "ctrlc 3.1.4 (registry+https://github.com/rust-lang/crates.io-index)", @@ -470,6 +476,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" "checksum aho-corasick 0.7.10 (registry+https://github.com/rust-lang/crates.io-index)" = "8716408b8bc624ed7f65d223ddb9ac2d044c0547b6fa4b0d554f3a9540496ada" "checksum ansi_term 0.11.0 (registry+https://github.com/rust-lang/crates.io-index)" = "ee49baf6cb617b853aa8d93bf420db2383fab46d314482ca2803b40d5fde979b" "checksum ansi_term 0.12.1 (registry+https://github.com/rust-lang/crates.io-index)" = "d52a9bb7ec0cf484c551830a7ce27bd20d67eac647e1befb56b0be4ee39a55d2" +"checksum anyhow 1.0.28 (registry+https://github.com/rust-lang/crates.io-index)" = "d9a60d744a80c30fcb657dfe2c1b22bcb3e814c1a1e3674f32bf5820b570fbff" "checksum atty 0.2.14 (registry+https://github.com/rust-lang/crates.io-index)" = "d9b39be18770d11421cdb1b9947a45dd3f37e93092cbf377614828a319d5fee8" "checksum autocfg 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)" = "f8aac770f1885fd7e387acedd76065302551364496e46b3dd00860b2f8359b9d" "checksum bitflags 1.2.1 (registry+https://github.com/rust-lang/crates.io-index)" = "cf1de2fe8c75bc145a2f577add951f8134889b4795d47466a54a5c846d691693" diff --git a/Cargo.toml b/Cargo.toml index 63365cf..f1452ab 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -45,6 +45,7 @@ ctrlc = "3.1" humantime = "2.0" lscolors = "0.7" globset = "0.4" +anyhow = "1.0" [dependencies.clap] version = "2.31.2" diff --git a/src/exec/mod.rs b/src/exec/mod.rs index 5c27491..84b1d8f 100644 --- a/src/exec/mod.rs +++ b/src/exec/mod.rs @@ -9,6 +9,7 @@ use std::path::{Path, PathBuf}; use std::process::{Command, Stdio}; use std::sync::{Arc, Mutex}; +use anyhow::{anyhow, Result}; use lazy_static::lazy_static; use regex::Regex; @@ -47,17 +48,19 @@ impl CommandTemplate { Self::build(input, ExecutionMode::OneByOne) } - pub fn new_batch(input: I) -> Result + pub fn new_batch(input: I) -> Result where I: IntoIterator, S: AsRef, { let cmd = Self::build(input, ExecutionMode::Batch); if cmd.number_of_tokens() > 1 { - return Err("Only one placeholder allowed for batch commands"); + return Err(anyhow!("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"); + return Err(anyhow!( + "First argument of exec-batch is expected to be a fixed executable" + )); } Ok(cmd) } diff --git a/src/main.rs b/src/main.rs index 3c2b7b2..b48b660 100644 --- a/src/main.rs +++ b/src/main.rs @@ -18,12 +18,14 @@ use std::process; use std::sync::Arc; use std::time; +use anyhow::{anyhow, Context, Result}; use atty::Stream; use globset::Glob; use lscolors::LsColors; use regex::bytes::{RegexBuilder, RegexSetBuilder}; use crate::exec::CommandTemplate; +use crate::exit_codes::ExitCode; use crate::filetypes::FileTypes; use crate::filter::{SizeFilter, TimeFilter}; use crate::options::Options; @@ -34,30 +36,31 @@ use crate::regex_helper::pattern_has_uppercase_char; #[global_allocator] static ALLOC: jemallocator::Jemalloc = jemallocator::Jemalloc; -fn main() { +fn run() -> Result { let matches = app::build_app().get_matches_from(env::args_os()); // Set the current working directory of the process if let Some(base_directory) = matches.value_of("base-directory") { let base_directory = Path::new(base_directory); if !filesystem::is_dir(base_directory) { - print_error_and_exit!( - "The '--base-directory' path ('{}') is not a directory.", + return Err(anyhow!( + "The '--base-directory' path '{}' is not a directory.", base_directory.to_string_lossy() - ); - } - if let Err(e) = env::set_current_dir(base_directory) { - print_error_and_exit!( - "Could not set '{}' as the current working directory: {}", - base_directory.to_string_lossy(), - e - ); + )); } + env::set_current_dir(base_directory).with_context(|| { + format!( + "Could not set '{}' as the current working directory.", + base_directory.to_string_lossy() + ) + })?; } let current_directory = Path::new("."); if !filesystem::is_dir(current_directory) { - print_error_and_exit!("Could not retrieve current directory (has it been deleted?)."); + return Err(anyhow!( + "Could not retrieve current directory (has it been deleted?)." + )); } // Get the search pattern @@ -71,15 +74,16 @@ fn main() { Some(paths) => paths .map(|path| { let path_buffer = PathBuf::from(path); - if !filesystem::is_dir(&path_buffer) { - print_error_and_exit!( - "'{}' is not a directory.", + if filesystem::is_dir(&path_buffer) { + Ok(path_buffer) + } else { + Err(anyhow!( + "Search path '{}' is not a directory.", path_buffer.to_string_lossy() - ); + )) } - path_buffer }) - .collect::>(), + .collect::>>()?, None => vec![current_directory.to_path_buf()], }; @@ -100,25 +104,20 @@ fn main() { && pattern.contains(std::path::MAIN_SEPARATOR) && filesystem::is_dir(Path::new(pattern)) { - print_error_and_exit!( + return Err(anyhow!( "The search pattern '{pattern}' contains a path-separation character ('{sep}') \ and will not lead to any search results.\n\n\ If you want to search for all files inside the '{pattern}' directory, use a match-all pattern:\n\n \ fd . '{pattern}'\n\n\ - Instead, if you want to search for the pattern in the full path, use:\n\n \ + Instead, if you want your pattern to match the full file path, use:\n\n \ fd --full-path '{pattern}'", pattern = pattern, sep = std::path::MAIN_SEPARATOR, - ); + )); } let pattern_regex = if matches.is_present("glob") { - let glob = match Glob::new(pattern) { - Ok(glob) => glob, - Err(e) => { - print_error_and_exit!("{}", e); - } - }; + let glob = Glob::new(pattern)?; glob.regex().to_owned() } else if matches.is_present("fixed-strings") { // Treat pattern as literal string if '--fixed-strings' is used @@ -149,48 +148,39 @@ fn main() { None }; - 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); - }) - }) - }) - .or_else(|| { - if matches.is_present("list-details") { - let color = matches.value_of("color").unwrap_or("auto"); - let color_arg = ["--color=", color].concat(); + let command = if let Some(args) = matches.values_of("exec") { + Some(CommandTemplate::new(args)) + } else if let Some(args) = matches.values_of("exec-batch") { + Some(CommandTemplate::new_batch(args)?) + } else if matches.is_present("list-details") { + let color = matches.value_of("color").unwrap_or("auto"); + let color_arg = ["--color=", color].concat(); - Some( - CommandTemplate::new_batch(&[ - "ls", - "-l", // long listing format - "--human-readable", // human readable file sizes - "--directory", // list directories themselves, not their contents - &color_arg, // enable colorized output, if enabled - ]) - .unwrap(), - ) - } else { - None - } - }); + Some( + CommandTemplate::new_batch(&[ + "ls", + "-l", // long listing format + "--human-readable", // human readable file sizes + "--directory", // list directories themselves, not their contents + &color_arg, // enable colorized output, if enabled + ]) + .unwrap(), + ) + } else { + None + }; - let size_limits: Vec = matches - .values_of("size") - .map(|v| { - v.map(|sf| { - if let Some(f) = SizeFilter::from_string(sf) { - return f; - } - print_error_and_exit!("'{}' is not a valid size constraint. See 'fd --help'.", sf); - }) - .collect() + let size_limits = if let Some(vs) = matches.values_of("size") { + vs.map(|sf| { + SizeFilter::from_string(sf).ok_or(anyhow!( + "'{}' is not a valid size constraint. See 'fd --help'.", + sf + )) }) - .unwrap_or_else(|| vec![]); + .collect::>>()? + } else { + vec![] + }; let now = time::SystemTime::now(); let mut time_constraints: Vec = Vec::new(); @@ -198,14 +188,20 @@ fn main() { if let Some(f) = TimeFilter::after(&now, t) { time_constraints.push(f); } else { - print_error_and_exit!("'{}' is not a valid date or duration. See 'fd --help'.", t); + return Err(anyhow!( + "'{}' is not a valid date or duration. See 'fd --help'.", + t + )); } } if let Some(t) = matches.value_of("changed-before") { if let Some(f) = TimeFilter::before(&now, t) { time_constraints.push(f); } else { - print_error_and_exit!("'{}' is not a valid date or duration. See 'fd --help'.", t); + return Err(anyhow!( + "'{}' is not a valid date or duration. See 'fd --help'.", + t + )); } } @@ -264,20 +260,17 @@ fn main() { file_types }), - extensions: matches.values_of("extension").map(|exts| { - let patterns = exts - .map(|e| e.trim_start_matches('.')) - .map(|e| format!(r".\.{}$", regex::escape(e))); - match RegexSetBuilder::new(patterns) - .case_insensitive(true) - .build() - { - Ok(re) => re, - Err(err) => { - print_error_and_exit!("{}", err.to_string()); - } - } - }), + extensions: matches + .values_of("extension") + .map(|exts| { + let patterns = exts + .map(|e| e.trim_start_matches('.')) + .map(|e| format!(r".\.{}$", regex::escape(e))); + RegexSetBuilder::new(patterns) + .case_insensitive(true) + .build() + }) + .transpose()?, command: command.map(Arc::new), exclude_patterns: matches .values_of("exclude") @@ -297,21 +290,33 @@ fn main() { .filter(|&n| n != 0), }; - match RegexBuilder::new(&pattern_regex) + let re = RegexBuilder::new(&pattern_regex) .case_insensitive(!config.case_sensitive) .dot_matches_new_line(true) .build() - { - Ok(re) => { - let exit_code = walk::scan(&dir_vec, Arc::new(re), Arc::new(config)); + .map_err(|e| { + anyhow!( + "{}\n\nNote: You can use the '--fixed-strings' option to search for a \ + literal string instead of a regular expression. Alternatively, you can \ + also use the '--glob' option to match on a glob pattern.", + e.to_string() + ) + })?; + + let exit_code = walk::scan(&dir_vec, Arc::new(re), Arc::new(config)); + + Ok(exit_code) +} + +fn main() { + let result = run(); + match result { + Ok(exit_code) => { process::exit(exit_code.into()); } Err(err) => { - print_error_and_exit!( - "{}\nHint: You can use the '--fixed-strings' option to search for a \ - literal string instead of a regular expression", - err.to_string() - ); + eprintln!("[fd error]: {}", err); + process::exit(1); } } }