better output 0u0

CHANGELOG.md

@@ -7,6 +7,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 ## Unreleased
 ### Changed
 - Capped help output (`-h`/`--help`) width at 120 characters max
+- Output is now sorted by filename - specifically, errors will appear first, followed by files that fif is unable to 
+  recommend an extension for, in order of filename, followed by files that fif knows how to rename, again in order 
+  of filename.
 ---
 
 ## v0.3.7 - 2021-09-25

src/findings.rs

@@ -1,6 +1,7 @@
 //! The [`Findings`] and [`ScanError`] structs, used for conveying whether a given file was able to be scanned and
 //! whether its MIME type could be inferred.
 
+use std::cmp::Ordering;
 use std::fmt::{Display, Formatter};
 use std::path::{Path, PathBuf};
 
@@ -12,7 +13,7 @@ use crate::files::mime_extension_lookup;
 use crate::String;
 
 /// Information about a scanned file.
-#[derive(Ord, PartialOrd, Eq, PartialEq)]
+#[derive(Eq, PartialEq)]
 pub struct Findings {
 	/// The location of the scanned file.
 	pub file: PathBuf,
@@ -22,6 +23,30 @@ pub struct Findings {
 	pub mime: Mime,
 }
 
+impl PartialOrd<Self> for Findings {
+	fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
+		Some(self.cmp(other))
+	}
+}
+
+impl Ord for Findings {
+	fn cmp(&self, other: &Self) -> Ordering {
+		// files with no recommended extension should appear first, so that fif outputs the "no known extension for x"
+		// comments before the "mv x y" instructions
+		// since fif doesn't output anything for valid files, the comparison will consider any comparison involving a
+		// valid Findings to be equal, avoiding the (somewhat) expensive call to recommended_extension. after all, since
+		// fif never displays valid files, it really doesn't matter what position they end up in.
+		if self.valid || other.valid {
+			return Ordering::Equal
+		}
+		match(self.recommended_extension(), other.recommended_extension()) {
+			(None, Some(_)) => Ordering::Greater,
+			(Some(_), None) => Ordering::Less,
+			_ => self.file.cmp(&other.file)
+		}
+	}
+}
+
 #[cfg(feature = "json")]
 impl serde::Serialize for Findings {
 	fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>

src/formats.rs

@@ -7,7 +7,7 @@ use std::os::unix::ffi::OsStrExt;
 use std::path::Path;
 
 use cfg_if::cfg_if;
-use itertools::Itertools;
+use itertools::{Either, Itertools};
 use snailquote::escape;
 
 use crate::findings::ScanError;
@@ -54,6 +54,21 @@ macro_rules! writablesln {
 #[doc(hidden)]
 type Entries<'a> = [Result<Findings, ScanError<'a>>];
 
+/// Splits the given [`Entries`] into [`Vec`]s of [`Findings`] and [`ScanError`]s. [`Findings`] are sorted by whether
+/// or not they have a known extension (unknown extensions coming first), and then by their filenames. [`ScanError`]s
+/// are sorted such that [`ScanError::File`]s come before [`ScanError::Mime`]s.
+#[inline]
+fn sort_entries<'a>(entries: &'a Entries) -> (Vec<&'a Findings>, Vec<&'a ScanError<'a>>) {
+	let (mut findings, mut errors): (Vec<_>, Vec<_>) = entries.iter().partition_map(|entry| match entry {
+		Ok(f) => Either::Left(f),
+		Err(e) => Either::Right(e)
+	});
+
+	findings.sort_unstable();
+	errors.sort_unstable();
+	(findings, errors)
+}
+
 #[derive(Debug, PartialEq)]
 pub enum Writable<'a> {
 	String(&'a str),
@@ -131,21 +146,7 @@ pub trait FormatSteps {
 	fn write_steps<W: Write>(&self, f: &mut W, entries: &Entries) -> io::Result<()> {
 		self.header(f, entries)?;
 
-		// output will be generated in the order:
+		let (findings, errors) = sort_entries(entries);
-		// - files that couldn't be read
-		// - files with no known mime type
-		// - files with no known extension
-		// - files with a known extension
-		// files that already have a correct extension won't be represented in the output.
-
-		// sort errors so unreadable files appear before files with unknown mimetypes - ScanError impls Ord such that
-		// ScanError::File > ScanError::Mime
-		let errors = entries.iter().filter_map(|e| e.as_ref().err()).sorted_unstable();
-		// sort files so that files with no known extension come before those with known extensions - None > Some("jpg")
-		let findings = entries
-			.iter()
-			.filter_map(|e| e.as_ref().ok())
-			.sorted_unstable_by(|a, b| b.recommended_extension().cmp(&a.recommended_extension()).reverse());
 
 		for error in errors {
 			match error {
@@ -338,18 +339,14 @@ pub struct Json;
 #[cfg(feature = "json")]
 impl Format for Json {
 	fn write_all<W: Write>(&self, f: &mut W, entries: &Entries) -> io::Result<()> {
-		use itertools::Either;
-
 		#[derive(serde::Serialize)]
 		struct SerdeEntries<'a> {
 			errors: &'a Vec<&'a ScanError<'a>>,
 			findings: &'a Vec<&'a Findings>,
 		}
 
-		let (errors, findings) = &entries.iter().partition_map(|entry| match entry {
+		let (findings, errors) = &sort_entries(entries);
-			Err(e) => Either::Left(e),
+
-			Ok(f) => Either::Right(f),
-		});
 
 		let result = serde_json::to_writer_pretty(f, &SerdeEntries { errors, findings });