Skip to main content

Merge pull request #59 from alexwlchan/debug

ID
c6519d8
date
2025-08-16 22:16:59+00:00
author
Alex Chan <alex@alexwlchan.net>
parents
8d95306, 2411b27
message
Merge pull request #59 from alexwlchan/debug

Print explicit errors when a directory cannot be deleted
changed files
6 files, 102 additions, 30 deletions

Changed files

CHANGELOG.md (970) → CHANGELOG.md (1318)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index e536447..6bfaff7 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,5 +1,11 @@
 # Changelog
 
+## v1.2.2 - 2025-08-16
+
+If `emptydir` tries to delete a directory but gets an error, it now prints that error to stderr. Previously the error would be silently ignored.
+
+This fixes a bug where emptydir could appear to do nothing -- it would report "no empty directories found", but actually it had found empty directories it was unable to delete.
+
 ## v1.2.1 - 2024-12-01
 
 Don't delete the `.git` directory or any subdirectories.

Cargo.lock (11081) → Cargo.lock (11081)

diff --git a/Cargo.lock b/Cargo.lock
index 1ba3e42..97a8a22 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -126,7 +126,7 @@ dependencies = [
 
 [[package]]
 name = "emptydir"
-version = "1.2.1"
+version = "1.2.2"
 dependencies = [
  "clap",
  "colored",

Cargo.toml (200) → Cargo.toml (200)

diff --git a/Cargo.toml b/Cargo.toml
index 702611d..749f28e 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "emptydir"
-version = "1.2.1"
+version = "1.2.2"
 edition = "2021"
 
 [dependencies]

src/can_be_deleted.rs (5954) → src/can_be_deleted.rs (5958)

diff --git a/src/can_be_deleted.rs b/src/can_be_deleted.rs
index 22a9c94..b0ac862 100644
--- a/src/can_be_deleted.rs
+++ b/src/can_be_deleted.rs
@@ -21,7 +21,7 @@ fn get_names_in_directory(dir: &Path) -> io::Result<HashSet<OsString>> {
 
 /// Returns True if this path any ancestor is a `.git` folder,
 /// False otherwise.
-pub fn is_in_git_folder(path: &Path) -> bool {
+fn is_in_git_repository(path: &Path) -> bool {
     path.ancestors()
         .any(|ancestor| ancestor.file_name().map_or(false, |name| name == ".git"))
 }
@@ -51,7 +51,7 @@ pub fn can_be_deleted(path: &Path) -> bool {
     //     fatal: not a git repository (or any of the parent directories): .git
     //
     // Skipping these folders is fine.
-    if is_in_git_folder(path) {
+    if is_in_git_repository(path) {
         return false;
     }
 

src/emptydir.rs (5257) → src/emptydir.rs (6817)

diff --git a/src/emptydir.rs b/src/emptydir.rs
index 19973f4..1324574 100644
--- a/src/emptydir.rs
+++ b/src/emptydir.rs
@@ -1,13 +1,20 @@
 use std::fs;
 use std::path::Path;
 
+use colored::*;
 use walkdir::WalkDir;
 
+#[derive(Debug, PartialEq)]
+pub struct EmptydirResult {
+    pub count_deleted: u32,
+    pub count_errors: u32,
+}
+
 /// Recurse through a given root directory, and delete any "empty" directories.
 ///
 /// Returns the number of directories deleted.
 ///
-pub fn emptydir(root: &Path) -> u32 {
+pub fn emptydir(root: &Path) -> EmptydirResult {
     let directories_to_delete = WalkDir::new(root)
         .contents_first(true)
         .into_iter()
@@ -16,6 +23,7 @@ pub fn emptydir(root: &Path) -> u32 {
         .filter(|e| crate::can_be_deleted::can_be_deleted(e.path()));
 
     let mut count_deleted: u32 = 0;
+    let mut count_errors: u32 = 0;
 
     for dir in directories_to_delete {
         match fs::remove_dir_all(dir.path()) {
@@ -23,7 +31,15 @@ pub fn emptydir(root: &Path) -> u32 {
                 println!("{}", dir.path().display());
                 count_deleted += 1;
             }
-            Err(_) => (),
+            Err(e) => {
+                let message = format!(
+                    "Tried to delete {}, but got error: {}",
+                    dir.path().display(),
+                    e
+                );
+                eprintln!("{}", message.red());
+                count_errors += 1;
+            }
         };
     }
 
@@ -32,22 +48,29 @@ pub fn emptydir(root: &Path) -> u32 {
     let mut current_parent = root.parent();
 
     while let Some(parent) = current_parent {
-        if crate::can_be_deleted::can_be_deleted(parent) {
-            match fs::remove_dir_all(parent) {
-                Ok(_) => {
-                    println!("{}", parent.display());
-                    count_deleted += 1;
-                }
-                Err(_) => (),
-            };
-
-            current_parent = parent.parent();
-        } else {
+        if !crate::can_be_deleted::can_be_deleted(parent) {
             break;
         }
+
+        match fs::remove_dir_all(parent) {
+            Ok(_) => {
+                println!("{}", parent.display());
+                count_deleted += 1;
+            }
+            Err(e) => {
+                let message = format!("Tried to delete {}, but got error: {}", parent.display(), e);
+                eprintln!("{}", message.red());
+                count_errors += 1;
+            }
+        };
+
+        current_parent = parent.parent();
     }
 
-    count_deleted
+    EmptydirResult {
+        count_deleted,
+        count_errors,
+    }
 }
 
 #[cfg(test)]
@@ -75,13 +98,25 @@ mod test_emptydir {
     #[test]
     fn it_doesnt_delete_my_do_not_backup() {
         let dir = Path::new("/Users/alexwlchan/Desktop/do not back up");
-        assert_eq!(emptydir(dir), 0);
+        assert_eq!(
+            emptydir(dir),
+            EmptydirResult {
+                count_deleted: 0,
+                count_errors: 0
+            }
+        );
     }
 
     #[test]
     fn it_doesnt_delete_a_non_existent_directory() {
         let dir = Path::new("/does/not/exist");
-        assert_eq!(emptydir(dir), 0);
+        assert_eq!(
+            emptydir(dir),
+            EmptydirResult {
+                count_deleted: 0,
+                count_errors: 0
+            }
+        );
     }
 
     #[test]
@@ -91,7 +126,13 @@ mod test_emptydir {
         // Create the directory, but don't put anything in it
         create_dir(&dir);
 
-        assert_eq!(emptydir(&dir), 1);
+        assert_eq!(
+            emptydir(&dir),
+            EmptydirResult {
+                count_deleted: 1,
+                count_errors: 0
+            }
+        );
         assert_eq!(dir.exists(), false);
     }
 
@@ -104,7 +145,13 @@ mod test_emptydir {
 
         create_file(&dir.join("greeting.txt"));
 
-        assert_eq!(emptydir(&dir), 0);
+        assert_eq!(
+            emptydir(&dir),
+            EmptydirResult {
+                count_deleted: 0,
+                count_errors: 0
+            }
+        );
         assert_eq!(dir.exists(), true);
         assert_eq!(dir.join("greeting.txt").exists(), true);
     }
@@ -139,7 +186,13 @@ mod test_emptydir {
 
         create_file(&dir.join(".DS_Store"));
 
-        assert_eq!(emptydir(&dir), 1);
+        assert_eq!(
+            emptydir(&dir),
+            EmptydirResult {
+                count_deleted: 1,
+                count_errors: 0
+            }
+        );
         assert_eq!(dir.exists(), false);
     }
 
@@ -152,7 +205,13 @@ mod test_emptydir {
         create_file(&dir.join(".DS_Store"));
         create_file(&dir.join("greeting.txt"));
 
-        assert_eq!(emptydir(&dir), 0);
+        assert_eq!(
+            emptydir(&dir),
+            EmptydirResult {
+                count_deleted: 0,
+                count_errors: 0
+            }
+        );
         assert!(dir.exists());
         assert!(dir.join("greeting.txt").exists());
     }
@@ -187,7 +246,13 @@ mod test_emptydir {
 
         create_file(&dir.join("greeting.txt"));
 
-        assert_eq!(emptydir(&dir), 1);
+        assert_eq!(
+            emptydir(&dir),
+            EmptydirResult {
+                count_deleted: 1,
+                count_errors: 0
+            }
+        );
         assert_eq!(dir.exists(), true);
         assert_eq!(subdir.exists(), false);
         assert!(dir.join("greeting.txt").exists());

src/main.rs (947) → src/main.rs (1065)

diff --git a/src/main.rs b/src/main.rs
index 142f95b..65ee544 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -21,15 +21,16 @@ fn main() -> Result<(), std::io::Error> {
     let cli = Cli::parse();
 
     let root = Path::new(&cli.root);
-    let count_deleted = emptydir::emptydir(root);
+    let result = emptydir::emptydir(root);
 
-    match count_deleted {
-        0 => println!("{}", "No empty directories found".blue()),
-        1 => println!("{}", "1 directory deleted".green()),
+    match (result.count_deleted, result.count_errors) {
+        (0, 0) => println!("{}", "No empty directories found".blue()),
+        (0, _) => println!("{}", "Unable to delete empty directories".red()),
+        (1, _) => println!("{}", "1 directory deleted".green()),
         _ => {
             let message = format!(
                 "{} directories deleted",
-                count_deleted.to_formatted_string(&Locale::en)
+                result.count_deleted.to_formatted_string(&Locale::en)
             );
             println!("{}", message.green());
         }