beeg beeg cleanup, set default prompt to error

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`
+  - By default, the user will be prompted only if fif encounters an error while renaming the file, or if renaming 
-    flag: `-p always` to be prompted each time (the default), `-p error` to be prompted on errors and when a file
+    the file would cause another file to be overwritten. This behaviour can be changed with the new `p`/`--prompt` 
-    would be overwritten by renaming, and `-p never` to disable prompting altogether - this behaves the same as
+    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`.

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"

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 }

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<String> {
 		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<PathBuf> {
-		if let Some(ext) = self.recommended_extension() {
+		self
-			Some(self.file.with_extension(ext.as_str()))
+			.recommended_extension()
-		} else {
+			.map(|ext| self.file.with_extension(ext.as_str()))
-			None
-		}
 	}
 }
 

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 {

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)]

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();
+	let ext_checks: HashMap<_, Option<&OsStr>> = hashmap![
-	ext_checks.insert(Path::new("test.txt"), Some(OsStr::new("txt")));
+	Path::new("test.txt") => Some(OsStr::new("txt")),
-	ext_checks.insert(Path::new("test.zip"), Some(OsStr::new("zip")));
+	Path::new("test.zip") => Some(OsStr::new("zip")),
-	ext_checks.insert(Path::new("test.tar.gz"), Some(OsStr::new("gz")));
+	Path::new("test.tar.gz") => Some(OsStr::new("gz")),
-	ext_checks.insert(Path::new("test."), Some(OsStr::new("")));
+	Path::new("test.") => Some(OsStr::new("")),
-	ext_checks.insert(Path::new("test"), None);
+	Path::new("test") => None,
-	ext_checks.insert(Path::new(".hidden"), 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();
+	let files = hashmap![
-	files.insert("test.jpg", JPEG_BYTES);
+		"test.jpg" => JPEG_BYTES,
-	files.insert("test.jpeg", JPEG_BYTES);
+		"test.jpeg" => JPEG_BYTES,
-	files.insert("test.png", PNG_BYTES);
+		"test.png" => PNG_BYTES,
-	files.insert("test.pdf", PDF_BYTES);
+		"test.pdf" => PDF_BYTES,
-	files.insert("test.zip", ZIP_BYTES);
+		"test.zip" => ZIP_BYTES,
-	files.insert("wrong.jpg", PNG_BYTES);
+		"wrong.jpg" => PNG_BYTES,
-	files.insert("ignore.fake_ext", ZIP_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_eq!(extensions, Some(btreeset!["ghi", "jkl"]));
-
-	assert!(!extensions.contains(&"abc"));
-	assert!(!extensions.contains(&"def"));
-	assert!(extensions.contains(&"ghi"));
-	assert!(extensions.contains(&"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, Some(btreeset!["flac"]));
-
-	assert_eq!(extensions.next(), Some("flac"), "Extensions should contain flac!");
-	assert_eq!(extensions.next(), None, "Too many extensions!");
 }
 
 #[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<Mime, i32> = HashMap::new();
+	let mut results: BTreeMap<Mime, i32> = 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();
+	let expected_results = hashmap![
-	expected_results.insert("-qqqqqqqq", LevelFilter::Off);
+		"-qqqqqqqq" => LevelFilter::Off,
-	expected_results.insert("-qqq", LevelFilter::Off);
+		"-qqq" => LevelFilter::Off,
-	expected_results.insert("-qq", LevelFilter::Error);
+		"-qq" => LevelFilter::Error,
-	expected_results.insert("-q", LevelFilter::Warn);
+		"-q" => LevelFilter::Warn,
-	expected_results.insert("-s", LevelFilter::Info);
+		"-s" => LevelFilter::Info,
-	expected_results.insert("-v", LevelFilter::Debug);
+		"-v" => LevelFilter::Debug,
-	expected_results.insert("-vv", LevelFilter::Trace);
+		"-vv" => LevelFilter::Trace,
-	expected_results.insert("-vvv", LevelFilter::Trace);
+		"-vvv" => LevelFilter::Trace,
-	expected_results.insert("-vvvvvvvv", LevelFilter::Trace);
+		"-vvvvvvvv" => LevelFilter::Trace,
+	];
 
 	for (flags, level) in expected_results {
 		assert_eq!(Parameters::parse_from(&["fif", flags]).default_verbosity(), level);