diff --git a/CHANGELOG.md b/CHANGELOG.md index 8b4e7b4..4865f07 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,9 +7,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), ## Unreleased ### Added - `--fix` mode - instead of outputting a shell script or text file, fif will rename the misnamed files for you! - - By default, the user will be prompted for each rename. This behaviour can be changed with the new `p`/`--prompt` - flag: `-p always` to be prompted each time (the default), `-p error` to be prompted on errors and when a file - would be overwritten by renaming, and `-p never` to disable prompting altogether - this behaves the same as + - By default, the user will be prompted only if fif encounters an error while renaming the file, or if renaming + the file would cause another file to be overwritten. This behaviour can be changed with the new `p`/`--prompt` + flag: `-p always` to be prompted each time, `-p error` to be prompted on errors and when a file would be + overwritten by renaming, and `-p never` to disable prompting altogether - this behaves the same as answering "yes" to every prompt. - The `--overwrite` flag must be specified along with `--fix` in order for fif to process renames that would cause an existing file to be overwritten. Without it, fif will never overwrite existing files, even with `-p always`. diff --git a/Cargo.lock b/Cargo.lock index 44822b6..d7f35fa 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -184,6 +184,7 @@ dependencies = [ "infer", "itertools", "log", + "maplit", "mime", "new_mime_guess", "num_cpus", @@ -308,6 +309,12 @@ dependencies = [ "cfg-if", ] +[[package]] +name = "maplit" +version = "1.0.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3e2e65a1a2e43cfcb47a895c4c8b10d1f4a61097f9f254f183aee60cad9c651d" + [[package]] name = "memchr" version = "2.4.1" diff --git a/Cargo.toml b/Cargo.toml index 271d1f5..e9547cb 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -37,6 +37,7 @@ serde = { version = "1.0", features = ["derive"], optional = true } serde_json = { version = "1.0", optional = true } bitflags = "~1.2.1" # 1.3+ requires Rust >= 1.46 num_cpus = { version = "1.13.0", optional = true } +maplit = "1.0.2" [target.'cfg(not(unix))'.dependencies] xdg-mime = { version = "0.3.3", optional = true } diff --git a/src/findings.rs b/src/findings.rs index db4dbfe..67f5940 100644 --- a/src/findings.rs +++ b/src/findings.rs @@ -24,16 +24,16 @@ pub struct Findings { } impl Findings { + /// Returns the recommended extension for this file, if known. pub fn recommended_extension(&self) -> Option { mime_extension_lookup(self.mime.essence_str().into()).map(|extensions| extensions[0].clone()) } + /// Returns the recommended path for this file - i.e. what it should be renamed to - if known. pub fn recommended_path(&self) -> Option { - if let Some(ext) = self.recommended_extension() { - Some(self.file.with_extension(ext.as_str())) - } else { - None - } + self + .recommended_extension() + .map(|ext| self.file.with_extension(ext.as_str())) } } diff --git a/src/main.rs b/src/main.rs index d243160..b58c666 100644 --- a/src/main.rs +++ b/src/main.rs @@ -34,7 +34,7 @@ use log::{debug, error, info, trace, warn, Level}; mod tests; #[doc(hidden)] -#[allow(clippy::cognitive_complexity)] +#[allow(clippy::cognitive_complexity, clippy::too_many_lines)] fn main() { let args: parameters::Parameters = parameters::Parameters::parse(); @@ -136,7 +136,7 @@ fn main() { .collect_vec(); if args.fix { - fn ask(message: String) -> bool { + fn ask(message: &str) -> bool { let mut buf = String::with_capacity(1); print!("{} [y/N] ", message); @@ -148,10 +148,10 @@ fn main() { error!("{}", e); exit(exitcode::IOERR) } - buf.starts_with("y") || buf.starts_with("Y") + buf.starts_with('y') || buf.starts_with('Y') } - let prompt = args.prompt.unwrap_or(Prompt::Always); + let prompt = args.prompt.unwrap_or(Prompt::Error); for f in findings { if let Some(rename_to) = f.recommended_path() { @@ -165,10 +165,10 @@ fn main() { // handles: --prompt never --overwrite // user specified --prompt never in conjunction with --overwrite, so always rename true - } else if prompt == Prompt::Error || ask(format!("Rename {:#?} to {:#?}?", &f.file, &rename_to)) { + } else if prompt == Prompt::Error || ask(&*format!("Rename {:#?} to {:#?}?", &f.file, &rename_to)) { // handles: --prompt error --overwrite, --prompt always --overwrite [y] // if the target exists, prompt before renaming; otherwise, just rename - !rename_to.exists() || ask(format!("Destination {:#?} already exists, overwrite?", rename_to)) + !rename_to.exists() || ask(&*format!("Destination {:#?} already exists, overwrite?", rename_to)) } else { // handles: --prompt always --overwrite [n] // user was prompted and replied "no" @@ -176,7 +176,9 @@ fn main() { } }; - if !will_rename { continue } + if !will_rename { + continue; + } loop { match std::fs::rename(&f.file, &rename_to) { @@ -188,7 +190,7 @@ fn main() { warn!("Couldn't rename {:#?} to {:#?}: {:#?}", f.file, rename_to, e); // if the user passed --prompt never, continue to the next file // otherwise, prompt user to retry move, retrying until the rename succeeds or they respond "N" - if prompt == Prompt::Never || !ask(format!("Error while renaming file: {:#?}. Try again?", e)) { + if prompt == Prompt::Never || !ask(&*format!("Error while renaming file: {:#?}. Try again?", e)) { break; } } @@ -196,7 +198,7 @@ fn main() { } } else { // no recommended name :c - info!("No known extension for file {:#?} of type {}", f.file, f.mime) + info!("No known extension for file {:#?} of type {}", f.file, f.mime); } } } else { diff --git a/src/parameters.rs b/src/parameters.rs index 5e50b8a..2ed0a5a 100644 --- a/src/parameters.rs +++ b/src/parameters.rs @@ -34,9 +34,12 @@ pub enum OutputFormat { #[derive(Clap, PartialEq, Debug)] pub enum Prompt { + /// Never prompt. Never, + /// Prompt only on errors, and on overwrites, if `--overwrite` is set. Error, - Always + /// Prompt for every rename. + Always, } #[derive(Clap, Debug)] diff --git a/src/tests/mod.rs b/src/tests/mod.rs index fe5407f..a15d518 100644 --- a/src/tests/mod.rs +++ b/src/tests/mod.rs @@ -1,4 +1,4 @@ -use std::collections::HashMap; +use std::collections::{BTreeMap, HashMap}; use std::ffi::OsStr; use std::path::{Path, PathBuf}; @@ -8,6 +8,7 @@ use fif::findings::Findings; use fif::formats::{Format, PowerShell, Shell}; use fif::mime_db::MimeDb; use fif::{String, MIMEDB}; +use maplit::{btreeset, hashmap}; use mime::{Mime, APPLICATION_OCTET_STREAM, APPLICATION_PDF, IMAGE_JPEG, IMAGE_PNG}; use crate::parameters::ExtensionSet; @@ -26,13 +27,14 @@ fn application_zip() -> Mime { #[test] /// Ensure that `extension_from_path` successfully returns the extension from a set of paths. fn get_ext() { - let mut ext_checks: HashMap<_, Option<&OsStr>> = HashMap::new(); - ext_checks.insert(Path::new("test.txt"), Some(OsStr::new("txt"))); - ext_checks.insert(Path::new("test.zip"), Some(OsStr::new("zip"))); - ext_checks.insert(Path::new("test.tar.gz"), Some(OsStr::new("gz"))); - ext_checks.insert(Path::new("test."), Some(OsStr::new(""))); - ext_checks.insert(Path::new("test"), None); - ext_checks.insert(Path::new(".hidden"), None); + let ext_checks: HashMap<_, Option<&OsStr>> = hashmap![ + Path::new("test.txt") => Some(OsStr::new("txt")), + Path::new("test.zip") => Some(OsStr::new("zip")), + Path::new("test.tar.gz") => Some(OsStr::new("gz")), + Path::new("test.") => Some(OsStr::new("")), + Path::new("test") => None, + Path::new(".hidden") => None, + ]; for (path, ext) in ext_checks { assert_eq!(path.extension(), ext); @@ -81,14 +83,15 @@ fn simple_directory() { // set of files to scan. all but the last files have magic numbers corresponding to their extension, except for // "wrong.jpg", which is actually a png. - let mut files = HashMap::new(); - files.insert("test.jpg", JPEG_BYTES); - files.insert("test.jpeg", JPEG_BYTES); - files.insert("test.png", PNG_BYTES); - files.insert("test.pdf", PDF_BYTES); - files.insert("test.zip", ZIP_BYTES); - files.insert("wrong.jpg", PNG_BYTES); - files.insert("ignore.fake_ext", ZIP_BYTES); + let files = hashmap![ + "test.jpg" => JPEG_BYTES, + "test.jpeg" => JPEG_BYTES, + "test.png" => PNG_BYTES, + "test.pdf" => PDF_BYTES, + "test.zip" => ZIP_BYTES, + "wrong.jpg" => PNG_BYTES, + "ignore.fake_ext" => ZIP_BYTES, + ]; let dir = tempdir().expect("Failed to create temporary directory."); set_current_dir(dir.path()).expect("Failed to change directory."); @@ -248,12 +251,7 @@ fn exclude_overrides() { let args: Parameters = Parameters::parse_from(vec!["fif", "-e", "abc,def,ghi,jkl", "-x", "abc,def"]); let extensions = args.extensions(); assert!(extensions.is_some(), "Extensions should be set!"); - let extensions = extensions.unwrap(); - - assert!(!extensions.contains(&"abc")); - assert!(!extensions.contains(&"def")); - assert!(extensions.contains(&"ghi")); - assert!(extensions.contains(&"jkl")); + assert_eq!(extensions, Some(btreeset!["ghi", "jkl"])); } #[test] @@ -263,10 +261,7 @@ fn exclude_set_overrides_includes() { let args: Parameters = Parameters::parse_from(vec!["fif", "-e", "jpg,flac", "-X", "images"]); let extensions = args.extensions(); assert!(extensions.is_some(), "Extensions should be set!"); - let mut extensions = extensions.unwrap().into_iter(); - - assert_eq!(extensions.next(), Some("flac"), "Extensions should contain flac!"); - assert_eq!(extensions.next(), None, "Too many extensions!"); + assert_eq!(extensions, Some(btreeset!["flac"])); } #[test] @@ -309,6 +304,10 @@ fn rejects_bad_args() { vec!["fif", "-e", ",,,,,"], // `-j` with a negative value: vec!["fif", "-j", "-1"], + // `--prompt` without `--fix`: + vec!["fif", "--prompt", "always"], + // `--overwrite` without `--fix`: + vec!["fif", "--overwrite"], ]; for test in &tests { @@ -323,9 +322,9 @@ fn identify_random_bytes() { use rand::RngCore; let mut rng = rand::thread_rng(); let mut bytes: [u8; BUF_SIZE * 2] = [0; BUF_SIZE * 2]; - let mut results: HashMap = HashMap::new(); + let mut results: BTreeMap = BTreeMap::new(); - for _ in 1..1000 { + for _ in 1..10000 { rng.fill_bytes(&mut bytes); if let Some(detected_type) = MIMEDB.get_type(&bytes) { *results.entry(detected_type).or_insert(0) += 1; @@ -450,16 +449,17 @@ fn verbosity() { "Failed to reject usage of both -q and -v!" ); - let mut expected_results = HashMap::new(); - expected_results.insert("-qqqqqqqq", LevelFilter::Off); - expected_results.insert("-qqq", LevelFilter::Off); - expected_results.insert("-qq", LevelFilter::Error); - expected_results.insert("-q", LevelFilter::Warn); - expected_results.insert("-s", LevelFilter::Info); - expected_results.insert("-v", LevelFilter::Debug); - expected_results.insert("-vv", LevelFilter::Trace); - expected_results.insert("-vvv", LevelFilter::Trace); - expected_results.insert("-vvvvvvvv", LevelFilter::Trace); + let expected_results = hashmap![ + "-qqqqqqqq" => LevelFilter::Off, + "-qqq" => LevelFilter::Off, + "-qq" => LevelFilter::Error, + "-q" => LevelFilter::Warn, + "-s" => LevelFilter::Info, + "-v" => LevelFilter::Debug, + "-vv" => LevelFilter::Trace, + "-vvv" => LevelFilter::Trace, + "-vvvvvvvv" => LevelFilter::Trace, + ]; for (flags, level) in expected_results { assert_eq!(Parameters::parse_from(&["fif", flags]).default_verbosity(), level);