rm: add the --progress option like with cp & mv

Author: sylvestre

Cargo.lock

@@ -40,6 +40,10 @@ packages.
 
 `mv` can display a progress bar when the `-g`/`--progress` flag is set.
 
+## `rm`
+
+`rm` can display a progress bar when the `-g`/`--progress` flag is set.
+
 ## `hashsum`
 
 This utility does not exist in GNU coreutils. `hashsum` is a utility that

docs/src/extensions.md

@@ -22,6 +22,7 @@ thiserror = { workspace = true }
 clap = { workspace = true }
 uucore = { workspace = true, features = ["fs", "parser", "safe-traversal"] }
 fluent = { workspace = true }
+indicatif = { workspace = true }
 
 [target.'cfg(unix)'.dependencies]
 libc = { workspace = true }

src/uu/rm/Cargo.toml

@@ -28,6 +28,10 @@ rm-help-preserve-root = do not remove '/' (default)
 rm-help-recursive = remove directories and their contents recursively
 rm-help-dir = remove empty directories
 rm-help-verbose = explain what is being done
+rm-help-progress = display a progress bar. Note: this feature is not supported by GNU coreutils.
+
+# Progress messages
+rm-progress-removing = Removing
 
 # Error messages
 rm-error-missing-operand = missing operand

src/uu/rm/locales/en-US.ftl

@@ -28,6 +28,10 @@ rm-help-preserve-root = ne pas supprimer '/' (par défaut)
 rm-help-recursive = supprimer les répertoires et leur contenu récursivement
 rm-help-dir = supprimer les répertoires vides
 rm-help-verbose = expliquer ce qui est fait
+rm-help-progress = afficher une barre de progression. Note : cette fonctionnalité n'est pas supportée par GNU coreutils.
+
+# Messages de progression
+rm-progress-removing = Suppression
 
 # Messages d'erreur
 rm-error-missing-operand = opérande manquant

src/uu/rm/locales/fr-FR.ftl

@@ -7,9 +7,10 @@
 
 use clap::builder::{PossibleValue, ValueParser};
 use clap::{Arg, ArgAction, Command, parser::ValueSource};
+use indicatif::{ProgressBar, ProgressStyle};
 use std::ffi::{OsStr, OsString};
 use std::fs::{self, Metadata};
-use std::io::{IsTerminal, stdin};
+use std::io::{self, IsTerminal, stdin};
 use std::ops::BitOr;
 #[cfg(unix)]
 use std::os::unix::ffi::OsStrExt;
@@ -108,6 +109,8 @@ pub struct Options {
     pub dir: bool,
     /// `-v`, `--verbose`
     pub verbose: bool,
+    /// `-g`, `--progress`
+    pub progress: bool,
     #[doc(hidden)]
     /// `---presume-input-tty`
     /// Always use `None`; GNU flag for testing use only
@@ -124,6 +127,7 @@ impl Default for Options {
             recursive: false,
             dir: false,
             verbose: false,
+            progress: false,
             __presume_input_tty: None,
         }
     }
@@ -139,6 +143,7 @@ static OPT_PROMPT_ALWAYS: &str = "prompt-always";
 static OPT_PROMPT_ONCE: &str = "prompt-once";
 static OPT_RECURSIVE: &str = "recursive";
 static OPT_VERBOSE: &str = "verbose";
+static OPT_PROGRESS: &str = "progress";
 static PRESUME_INPUT_TTY: &str = "-presume-input-tty";
 
 static ARG_FILES: &str = "files";
@@ -191,6 +196,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
         recursive: matches.get_flag(OPT_RECURSIVE),
         dir: matches.get_flag(OPT_DIR),
         verbose: matches.get_flag(OPT_VERBOSE),
+        progress: matches.get_flag(OPT_PROGRESS),
         __presume_input_tty: if matches.get_flag(PRESUME_INPUT_TTY) {
             Some(true)
         } else {
@@ -309,6 +315,13 @@ pub fn uu_app() -> Command {
                 .help(translate!("rm-help-verbose"))
                 .action(ArgAction::SetTrue),
         )
+        .arg(
+            Arg::new(OPT_PROGRESS)
+                .short('g')
+                .long(OPT_PROGRESS)
+                .help(translate!("rm-help-progress"))
+                .action(ArgAction::SetTrue),
+        )
         // From the GNU source code:
         // This is solely for testing.
         // Do not document.
@@ -333,6 +346,63 @@ pub fn uu_app() -> Command {
         )
 }
 
+/// Count the total number of files and directories to be deleted.
+/// This function recursively counts all files and directories that will be processed.
+/// Files are not deduplicated when appearing in multiple sources. If `recursive` is set to `false`, the
+/// directories in `paths` will be ignored.
+fn count_files(paths: &[&OsStr], recursive: bool) -> io::Result<u64> {
+    let mut total = 0;
+    for p in paths {
+        let path = Path::new(p);
+        match fs::symlink_metadata(path) {
+            Ok(md) => {
+                if md.is_dir() && !is_symlink_dir(&md) {
+                    if recursive {
+                        total += count_files_in_directory(path)?;
+                    }
+                } else {
+                    total += 1; // Count this file/symlink
+                }
+            }
+            Err(_) => {
+                // If we can't access the file, skip it for counting
+                // This matches the behavior where -f suppresses errors for missing files
+            }
+        }
+    }
+    Ok(total)
+}
+
+/// A helper for `count_files` specialized for directories.
+fn count_files_in_directory(p: &Path) -> io::Result<u64> {
+    let mut total = 1; // Count the directory itself
+
+    match fs::read_dir(p) {
+        Ok(entries) => {
+            for entry in entries.flatten() {
+                let path = entry.path();
+                match entry.file_type() {
+                    Ok(file_type) => {
+                        if file_type.is_dir() {
+                            total += count_files_in_directory(&path)?;
+                        } else {
+                            total += 1; // Count this file
+                        }
+                    }
+                    Err(_) => {
+                        // Skip files we can't access
+                    }
+                }
+            }
+        }
+        Err(_) => {
+            // Skip directories we can't read
+        }
+    }
+
+    Ok(total)
+}
+
 // TODO: implement one-file-system (this may get partially implemented in walkdir)
 /// Remove (or unlink) the given files
 ///
@@ -343,17 +413,35 @@ pub fn uu_app() -> Command {
 pub fn remove(files: &[&OsStr], options: &Options) -> bool {
     let mut had_err = false;
 
+    // Create progress bar if requested
+    let progress_bar = if options.progress {
+        count_files(files, options.recursive)
+            .ok()
+            .map(|total_files| {
+                ProgressBar::new(total_files)
+                    .with_style(
+                        ProgressStyle::with_template(
+                            "{msg}: [{elapsed_precise}] {wide_bar} {pos:>7}/{len:7} files",
+                        )
+                        .unwrap(),
+                    )
+                    .with_message(translate!("rm-progress-removing"))
+            })
+    } else {
+        None
+    };
+
     for filename in files {
         let file = Path::new(filename);
 
         had_err = match file.symlink_metadata() {
             Ok(metadata) => {
                 if metadata.is_dir() {
-                    handle_dir(file, options)
+                    handle_dir(file, options, progress_bar.as_ref())
                 } else if is_symlink_dir(&metadata) {
-                    remove_dir(file, options)
+                    remove_dir(file, options, progress_bar.as_ref())
                 } else {
-                    remove_file(file, options)
+                    remove_file(file, options, progress_bar.as_ref())
                 }
             }
 
@@ -376,6 +464,10 @@ pub fn remove(files: &[&OsStr], options: &Options) -> bool {
         .bitor(had_err);
     }
 
+    if let Some(pb) = progress_bar {
+        pb.finish();
+    }
+
     had_err
 }
 
@@ -482,7 +574,7 @@ fn safe_remove_dir_recursive_impl(path: &Path, dir_fd: &DirFd, options: &Options
     // Read directory entries using safe traversal
     let entries = match dir_fd.read_dir() {
         Ok(entries) => entries,
-        Err(e) if e.kind() == std::io::ErrorKind::PermissionDenied => {
+        Err(e) if e.kind() == io::ErrorKind::PermissionDenied => {
             // This is not considered an error - just like the original
             return false;
         }
@@ -570,15 +662,19 @@ fn safe_remove_dir_recursive_impl(path: &Path, dir_fd: &DirFd, options: &Options
 /// directory, remove all of its entries recursively and then remove the
 /// directory itself. In case of an error, print the error message to
 /// `stderr` and return `true`. If there were no errors, return `false`.
-fn remove_dir_recursive(path: &Path, options: &Options) -> bool {
+fn remove_dir_recursive(
+    path: &Path,
+    options: &Options,
+    progress_bar: Option<&ProgressBar>,
+) -> bool {
     // Base case 1: this is a file or a symbolic link.
     //
     // The symbolic link case is important because it could be a link to
     // a directory and we don't want to recurse. In particular, this
     // avoids an infinite recursion in the case of a link to the current
     // directory, like `ln -s . link`.
     if !path.is_dir() || path.is_symlink() {
-        return remove_file(path, options);
+        return remove_file(path, options, progress_bar);
     }
 
     // Base case 2: this is a non-empty directory, but the user
@@ -622,7 +718,7 @@ fn remove_dir_recursive(path: &Path, options: &Options) -> bool {
     // Recursive case: this is a directory.
     let mut error = false;
     match fs::read_dir(path) {
-        Err(e) if e.kind() == std::io::ErrorKind::PermissionDenied => {
+        Err(e) if e.kind() == io::ErrorKind::PermissionDenied => {
             // This is not considered an error.
         }
         Err(_) => error = true,
@@ -631,7 +727,8 @@ fn remove_dir_recursive(path: &Path, options: &Options) -> bool {
                 match entry {
                     Err(_) => error = true,
                     Ok(entry) => {
-                        let child_error = remove_dir_recursive(&entry.path(), options);
+                        let child_error =
+                            remove_dir_recursive(&entry.path(), options, progress_bar);
                         error = error || child_error;
                     }
                 }
@@ -675,7 +772,7 @@ fn remove_dir_recursive(path: &Path, options: &Options) -> bool {
     error
 }
 
-fn handle_dir(path: &Path, options: &Options) -> bool {
+fn handle_dir(path: &Path, options: &Options, progress_bar: Option<&ProgressBar>) -> bool {
     let mut had_err = false;
 
     let path = clean_trailing_slashes(path);
@@ -689,9 +786,9 @@ fn handle_dir(path: &Path, options: &Options) -> bool {
 
     let is_root = path.has_root() && path.parent().is_none();
     if options.recursive && (!is_root || !options.preserve_root) {
-        had_err = remove_dir_recursive(path, options);
+        had_err = remove_dir_recursive(path, options, progress_bar);
     } else if options.dir && (!is_root || !options.preserve_root) {
-        had_err = remove_dir(path, options).bitor(had_err);
+        had_err = remove_dir(path, options, progress_bar).bitor(had_err);
     } else if options.recursive {
         show_error!("{}", RmError::DangerousRecursiveOperation);
         show_error!("{}", RmError::UseNoPreserveRoot);
@@ -710,7 +807,7 @@ fn handle_dir(path: &Path, options: &Options) -> bool {
 /// Remove the given directory, asking the user for permission if necessary.
 ///
 /// Returns true if it has encountered an error.
-fn remove_dir(path: &Path, options: &Options) -> bool {
+fn remove_dir(path: &Path, options: &Options, progress_bar: Option<&ProgressBar>) -> bool {
     // Ask the user for permission.
     if !prompt_dir(path, options) {
         return false;
@@ -726,6 +823,11 @@ fn remove_dir(path: &Path, options: &Options) -> bool {
     }
 
     // Try to remove the directory.
+    // Update progress bar for directory removal
+    if let Some(pb) = progress_bar {
+        pb.inc(1);
+    }
+
     match fs::remove_dir(path) {
         Ok(_) => {
             if options.verbose {
@@ -745,8 +847,13 @@ fn remove_dir(path: &Path, options: &Options) -> bool {
     }
 }
 
-fn remove_file(path: &Path, options: &Options) -> bool {
+fn remove_file(path: &Path, options: &Options, progress_bar: Option<&ProgressBar>) -> bool {
     if prompt_file(path, options) {
+        // Update progress bar before removing the file
+        if let Some(pb) = progress_bar {
+            pb.inc(1);
+        }
+
         match fs::remove_file(path) {
             Ok(_) => {
                 if options.verbose {
@@ -757,7 +864,7 @@ fn remove_file(path: &Path, options: &Options) -> bool {
                 }
             }
             Err(e) => {
-                if e.kind() == std::io::ErrorKind::PermissionDenied {
+                if e.kind() == io::ErrorKind::PermissionDenied {
                     // GNU compatibility (rm/fail-eacces.sh)
                     show_error!(
                         "{}",

src/uu/rm/src/rm.rs

@@ -1078,3 +1078,45 @@ fn test_rm_recursive_long_path_safe_traversal() {
     // Verify the directory is completely removed
     assert!(!at.dir_exists("rm_deep"));
 }
+
+#[test]
+fn test_progress_flag() {
+    let (at, mut ucmd) = at_and_ucmd!();
+    let file = "test_rm_progress_file";
+
+    at.touch(file);
+
+    // Test that -g/--progress flag is accepted
+    ucmd.arg("-g").arg(file).succeeds();
+
+    assert!(!at.file_exists(file));
+}
+
+#[test]
+fn test_progress_with_recursive() {
+    let (at, mut ucmd) = at_and_ucmd!();
+
+    at.mkdir("test_dir");
+    at.touch("test_dir/file1");
+    at.touch("test_dir/file2");
+    at.mkdir("test_dir/subdir");
+    at.touch("test_dir/subdir/file3");
+
+    // Test progress with recursive removal
+    ucmd.arg("-rg").arg("test_dir").succeeds();
+
+    assert!(!at.dir_exists("test_dir"));
+}
+
+#[test]
+fn test_progress_with_verbose() {
+    let (at, mut ucmd) = at_and_ucmd!();
+    let file = "test_rm_progress_verbose_file";
+
+    at.touch(file);
+
+    // Test that progress and verbose work together
+    ucmd.arg("-gv").arg(file).succeeds().stdout_contains(file);
+
+    assert!(!at.file_exists(file));
+}