refactor: implement thiserror for better error handling

Cargo.lock

@@ -131,6 +131,7 @@ version = "0.1.0"
 dependencies = [
  "clap",
  "regex",
+ "thiserror",
  "yansi",
 ]
 
@@ -204,6 +205,26 @@ dependencies = [
  "unicode-ident",
 ]
 
+[[package]]
+name = "thiserror"
+version = "2.0.12"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "567b8a2dae586314f7be2a752ec7474332959c6460e02bde30d702a66d488708"
+dependencies = [
+ "thiserror-impl",
+]
+
+[[package]]
+name = "thiserror-impl"
+version = "2.0.12"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "7f7cf42b4507d8ea322120659672cf1b9dbb93f8f2d4ecfd6e51350ff5b17a1d"
+dependencies = [
+ "proc-macro2",
+ "quote",
+ "syn",
+]
+
 [[package]]
 name = "unicode-ident"
 version = "1.0.18"

Cargo.toml

@@ -6,4 +6,5 @@ edition = "2024"
 [dependencies]
 clap = { version = "4.5.37", features = ["derive"] }
 regex = "1.11.1"
+thiserror = "2.0.12"
 yansi = "1.0.1"

src/main.rs

@@ -4,12 +4,35 @@ use regex::Regex;
 use std::{
     collections::{HashMap, HashSet},
     process::Command,
-    string::{String, ToString},
+    string::String,
     sync::OnceLock,
     thread,
 };
+use thiserror::Error;
 use yansi::Paint;
 
+/// Application errors with thiserror
+#[derive(Debug, Error)]
+enum AppError {
+    #[error("Command failed: {0}")]
+    CommandFailed(String),
+
+    #[error("Failed to decode command output: {0}")]
+    CommandOutputError(#[from] std::str::Utf8Error),
+
+    #[error("Failed to parse data: {0}")]
+    ParseError(String),
+
+    #[error("Regex error: {0}")]
+    RegexError(#[from] regex::Error),
+
+    #[error("IO error: {0}")]
+    IoError(#[from] std::io::Error),
+}
+
+// Use type alias for Result with our custom error type
+type Result<T> = std::result::Result<T, AppError>;
+
 #[derive(Parser, Debug)]
 #[command(name = "Nix not Python diff tool")]
 #[command(version = "1.0")]
@@ -28,6 +51,7 @@ struct Args {
     closure_size: bool,
 }
 
+#[derive(Debug, Clone)]
 struct Package<'a> {
     name: &'a str,
     versions: HashSet<&'a str>,
@@ -35,6 +59,22 @@ struct Package<'a> {
     is_dep: bool,
 }
 
+impl<'a> Package<'a> {
+    fn new(name: &'a str, version: &'a str, is_dep: bool) -> Self {
+        let mut versions = HashSet::new();
+        versions.insert(version);
+        Self {
+            name,
+            versions,
+            is_dep,
+        }
+    }
+
+    fn add_version(&mut self, version: &'a str) {
+        self.versions.insert(version);
+    }
+}
+
 fn main() {
     let args = Args::parse();
 
@@ -43,7 +83,7 @@ fn main() {
     println!("<<< {}", args.path.to_string_lossy());
     println!(">>> {}", args.path2.to_string_lossy());
 
-    // handles to the threads collecting closure size information
+    // Handles to the threads collecting closure size information
     // We do this as early as possible because nix is slow.
     let closure_size_handles = if args.closure_size {
         let path = args.path.clone();
@@ -56,30 +96,61 @@ fn main() {
         None
     };
 
-    let package_list_pre = get_packages(&args.path);
+    // Get package lists and handle potential errors
-    let package_list_post = get_packages(&args.path2);
+    let package_list_pre = match get_packages(&args.path) {
+        Ok(packages) => packages,
+        Err(e) => {
+            eprintln!("Error getting packages from path {}: {}", args.path.display(), e);
+            Vec::new()
+        }
+    };
+
+    let package_list_post = match get_packages(&args.path2) {
+        Ok(packages) => packages,
+        Err(e) => {
+            eprintln!("Error getting packages from path {}: {}", args.path2.display(), e);
+            Vec::new()
+        }
+    };
 
     // Map from packages of the first closure to their version
     let mut pre = HashMap::<&str, HashSet<&str>>::new();
     let mut post = HashMap::<&str, HashSet<&str>>::new();
 
     for p in &package_list_pre {
-        let (name, version) = get_version(&**p);
+        match get_version(&**p) {
+            Ok((name, version)) => {
                 pre.entry(name).or_default().insert(version);
+            },
+            Err(e) => {
+                if cfg!(debug_assertions) {
+                    eprintln!("Error parsing package version: {e}");
                 }
+            }
+        }
+    }
+
     for p in &package_list_post {
-        let (name, version) = get_version(&**p);
+        match get_version(&**p) {
+            Ok((name, version)) => {
                 post.entry(name).or_default().insert(version);
+            },
+            Err(e) => {
+                if cfg!(debug_assertions) {
+                    eprintln!("Error parsing package version: {e}");
+                }
+            }
+        }
     }
 
     // Compare the package names of both versions
     let pre_keys: HashSet<&str> = pre.keys().copied().collect();
     let post_keys: HashSet<&str> = post.keys().copied().collect();
 
-    // difference gives us added and removed packages
+    // Difference gives us added and removed packages
     let added: HashSet<&str> = &post_keys - &pre_keys;
     let removed: HashSet<&str> = &pre_keys - &post_keys;
-    // get the intersection of the package names for version changes
+    // Get the intersection of the package names for version changes
     let changed: HashSet<&str> = &pre_keys & &post_keys;
 
     println!("Difference between the two generations:");
@@ -93,12 +164,15 @@ fn main() {
 
     if let Some((pre_handle, post_handle)) = closure_size_handles {
         match (pre_handle.join(), post_handle.join()) {
-            (Ok(pre_size), Ok(post_size)) => {
+            (Ok(Ok(pre_size)), Ok(Ok(post_size))) => {
                 println!("{}", "Closure Size:".underline().bold());
                 println!("Before: {pre_size} MiB");
                 println!("After: {post_size} MiB");
                 println!("Difference: {} MiB", post_size - pre_size);
             }
+            (Ok(Err(e)), _) | (_, Ok(Err(e))) => {
+                eprintln!("Error getting closure size: {e}");
+            }
             _ => {
                 eprintln!("Error: Failed to get closure size information due to a thread error");
             }
@@ -106,56 +180,44 @@ fn main() {
     }
 }
 
-// gets the packages in a closure
+/// Gets the packages in a closure
-fn get_packages(path: &std::path::Path) -> Vec<String> {
+fn get_packages(path: &std::path::Path) -> Result<Vec<String>> {
-    // get the nix store paths using nix-store --query --references <path>
+    // Get the nix store paths using `nix-store --query --references <path>``
     let output = Command::new("nix-store")
         .arg("--query")
         .arg("--references")
         .arg(path.join("sw"))
-        .output();
+        .output()?;
 
-    match output {
+    if !output.status.success() {
-        Ok(query) => {
+        let stderr = String::from_utf8_lossy(&output.stderr);
-            match str::from_utf8(&query.stdout) {
+        return Err(AppError::CommandFailed(format!(
-                Ok(list) => list.lines().map(ToString::to_string).collect(),
+            "nix-store command failed: {stderr}"
-                Err(e) => {
+        )));
-                    eprintln!("Error decoding command output: {}", e);
-                    Vec::new()
-                }
-            }
-        }
-        Err(e) => {
-            eprintln!("Error executing nix-store command: {}", e);
-            Vec::new()
-        }
-    }
     }
 
-// gets the dependencies of the packages in a closure
+    let list = str::from_utf8(&output.stdout)?;
-fn get_dependencies(path: &std::path::Path) -> Vec<String> {
+    Ok(list.lines().map(str::to_owned).collect())
-    // get the nix store paths using nix-store --query --references <path>
+}
+
+/// Gets the dependencies of the packages in a closure
+fn get_dependencies(path: &std::path::Path) -> Result<Vec<String>> {
+    // Get the nix store paths using `nix-store --query --requisites <path>``
     let output = Command::new("nix-store")
         .arg("--query")
         .arg("--requisites")
         .arg(path.join("sw"))
-        .output();
+        .output()?;
 
-    match output {
+    if !output.status.success() {
-        Ok(query) => {
+        let stderr = String::from_utf8_lossy(&output.stderr);
-            match str::from_utf8(&query.stdout) {
+        return Err(AppError::CommandFailed(format!(
-                Ok(list) => list.lines().map(ToString::to_string).collect(),
+            "nix-store command failed: {stderr}"
-                Err(e) => {
+        )));
-                    eprintln!("Error decoding command output: {}", e);
-                    Vec::new()
-                }
-            }
-        }
-        Err(e) => {
-            eprintln!("Error executing nix-store command: {}", e);
-            Vec::new()
-        }
     }
+
+    let list = str::from_utf8(&output.stdout)?;
+    Ok(list.lines().map(str::to_owned).collect())
 }
 
 // Returns a reference to the compiled regex pattern.
@@ -168,8 +230,8 @@ fn store_path_regex() -> &'static Regex {
     })
 }
 
-fn get_version<'a>(pack: impl Into<&'a str>) -> (&'a str, &'a str) {
+// Parse the nix store path to extract package name and version
-    // funny regex to split a nix store path into its name and its version.
+fn get_version<'a>(pack: impl Into<&'a str>) -> Result<(&'a str, &'a str)> {
     let path = pack.into();
 
     // Match the regex against the input
@@ -177,10 +239,19 @@ fn get_version<'a>(pack: impl Into<&'a str>) -> (&'a str, &'a str) {
         // Handle potential missing captures safely
         let name = cap.get(1).map_or("", |m| m.as_str());
         let version = cap.get(2).map_or("", |m| m.as_str());
-        return (name, version);
+
+        if name.is_empty() || version.is_empty() {
+            return Err(AppError::ParseError(format!(
+                "Failed to extract name or version from path: {path}"
+            )));
         }
 
-    ("", "")
+        return Ok((name, version));
+    }
+
+    Err(AppError::ParseError(format!(
+        "Path does not match expected nix store format: {path}"
+    )))
 }
 
 fn check_nix_available() -> bool {
@@ -191,62 +262,45 @@ fn check_nix_available() -> bool {
     nix_available.is_some() && nix_query_available.is_some()
 }
 
-fn get_closure_size(path: &std::path::Path) -> i64 {
+fn get_closure_size(path: &std::path::Path) -> Result<i64> {
     // Run nix path-info command to get closure size
-    match Command::new("nix")
+    let output = Command::new("nix")
         .arg("path-info")
         .arg("--closure-size")
         .arg(path.join("sw"))
-        .output()
+        .output()?;
-    {
+
-        Ok(output) if output.status.success() => {
+    if !output.status.success() {
-            // Parse command output to extract the size
+        let stderr = String::from_utf8_lossy(&output.stderr);
-            match str::from_utf8(&output.stdout) {
+        return Err(AppError::CommandFailed(format!(
-                Ok(stdout) => {
+            "nix path-info command failed: {stderr}"
-                    // Parse the last word in the output as an integer
+        )));
+    }
+
+    let stdout = str::from_utf8(&output.stdout)?;
+
+    // Parse the last word in the output as an integer (in bytes)
     stdout
         .split_whitespace()
         .last()
-                        .and_then(|s| s.parse::<i64>().ok())
+        .ok_or_else(|| AppError::ParseError("Unexpected output format from nix path-info".into()))?
-                        .map_or_else(
+        .parse::<i64>()
-                            || {
+        .map_err(|e| AppError::ParseError(format!("Failed to parse size value: {e}")))
-                                eprintln!("Failed to parse closure size from output: {}", stdout);
+        .map(|size| size / 1024 / 1024) // Convert to MiB
-                                0
-                            },
-                            |size| size / 1024 / 1024, // Convert to MiB
-                        )
-                }
-                Err(e) => {
-                    eprintln!("Error decoding command output: {}", e);
-                    0
-                }
-            }
-        }
-        Ok(output) => {
-            // Command ran but returned an error
-            match str::from_utf8(&output.stderr) {
-                Ok(stderr) if !stderr.is_empty() => {
-                    eprintln!("nix path-info command failed: {}", stderr);
-                }
-                _ => {
-                    eprintln!("nix path-info command failed with status: {}", output.status);
-                }
-            }
-            0
-        }
-        Err(e) => {
-            // Command failed to run
-            eprintln!("Failed to execute nix path-info command: {}", e);
-            0
-        }
-    }
 }
 
 fn print_added(set: HashSet<&str>, post: &HashMap<&str, HashSet<&str>>) {
     println!("{}", "Packages added:".underline().bold());
-    for p in set {
+
-        let posts = post.get(p);
+    // Use sorted output
-        if let Some(ver) = posts {
+    let mut sorted: Vec<_> = set.iter()
+        .filter_map(|p| post.get(p).map(|ver| (*p, ver)))
+        .collect();
+
+    // Sort by package name for consistent output
+    sorted.sort_by(|(a, _), (b, _)| a.cmp(b));
+
+    for (p, ver) in sorted {
         let version_str = ver.iter().copied().collect::<Vec<_>>().join(" ");
         println!(
             "{} {} {} {}",
@@ -257,12 +311,12 @@ fn print_added(set: HashSet<&str>, post: &HashMap<&str, HashSet<&str>>) {
         );
     }
 }
-}
+
 fn print_removed(set: HashSet<&str>, pre: &HashMap<&str, HashSet<&str>>) {
     println!("{}", "Packages removed:".underline().bold());
-    for p in set {
+    set.iter()
-        let pre = pre.get(p);
+        .filter_map(|p| pre.get(p).map(|ver| (*p, ver)))
-        if let Some(ver) = pre {
+        .for_each(|(p, ver)| {
             let version_str = ver.iter().copied().collect::<Vec<_>>().join(" ");
             println!(
                 "{} {} {} {}",
@@ -271,26 +325,29 @@ fn print_removed(set: HashSet<&str>, pre: &HashMap<&str, HashSet<&str>>) {
                 "@".yellow(),
                 version_str.blue()
             );
+        });
 }
-    }
+
-}
 fn print_changes(
     set: HashSet<&str>,
     pre: &HashMap<&str, HashSet<&str>>,
     post: &HashMap<&str, HashSet<&str>>,
 ) {
     println!("{}", "Version changes:".underline().bold());
-    for p in set {
+    set.iter()
-        if p.is_empty() {
+        .filter(|p| !p.is_empty())
-            continue;
+        .filter_map(|p| {
+            match (pre.get(p), post.get(p)) {
+                (Some(ver_pre), Some(ver_post)) if ver_pre != ver_post => {
+                    Some((p, ver_pre, ver_post))
                 }
-
+                _ => None,
-        // We should handle the case where the package might not exist in one of the maps
+            }
-        if let (Some(ver_pre), Some(ver_post)) = (pre.get(p), post.get(p)) {
+        })
+        .for_each(|(p, ver_pre, ver_post)| {
             let version_str_pre = ver_pre.iter().copied().collect::<Vec<_>>().join(" ");
             let version_str_post = ver_post.iter().copied().collect::<Vec<_>>().join(", ");
 
-            if ver_pre != ver_post {
             println!(
                 "{} {} {} {} ~> {}",
                 "[C:]".bold().bright_yellow(),
@@ -299,7 +356,5 @@ fn print_changes(
                 version_str_pre.magenta(),
                 version_str_post.blue()
             );
-            }
+        });
-        }
-    }
 }