Merge pull request #297 from ouch-org/fix-fatal-error-if-read-fails-when-compressing

Fix fatal error if read fails when compressing

src/commands/compress.rs

@@ -19,13 +19,16 @@ use crate::{
     QuestionAction, QuestionPolicy, BUFFER_CAPACITY,
 };
 
-// Compress files into an `output_file`
-//
-// - `files`: is the list of paths to be compressed: ["dir/file1.txt", "dir/file2.txt"]
-// - `extensions`: contains each compression format necessary for compressing, example: [Tar, Gz] (in compression order)
-// - `output_file` is the resulting compressed file name, example: "compressed.tar.gz"
-//
-// Returns Ok(true) if compressed all files successfully, and Ok(false) if user opted to skip files
+/// Compress files into `output_file`.
+///
+/// # Arguments:
+/// - `files`: is the list of paths to be compressed: ["dir/file1.txt", "dir/file2.txt"]
+/// - `extensions`: is a list of compression formats for compressing, example: [Tar, Gz] (in compression order)
+/// - `output_file` is the resulting compressed file name, example: "archive.tar.gz"
+///
+/// # Return value
+/// - Returns `Ok(true)` if compressed all files normally.
+/// - Returns `Ok(false)` if user opted to abort compression mid-way.
 pub fn compress_files(
     files: Vec<PathBuf>,
     extensions: Vec<Extension>,

src/commands/mod.rs

@@ -9,7 +9,6 @@ use std::{
     path::{Path, PathBuf},
 };
 
-use fs_err as fs;
 use utils::colors;
 
 use crate::{
@@ -183,22 +182,19 @@ pub fn run(
                 // about whether the command succeeded without such a message
                 info!(accessible, "Successfully compressed '{}'.", to_utf(&output_path));
             } else {
-                // If Ok(false) or Err() occurred, delete incomplete file
-                // Print an extra alert message pointing out that we left a possibly
-                // CORRUPTED FILE at `output_path`
-                if let Err(err) = fs::remove_file(&output_path) {
+                // If Ok(false) or Err() occurred, delete incomplete file at `output_path`
+                //
+                // if deleting fails, print an extra alert message pointing
+                // out that we left a possibly CORRUPTED file at `output_path`
+                if utils::remove_file_or_dir(&output_path).is_err() {
                     eprintln!("{red}FATAL ERROR:\n", red = *colors::RED);
-                    eprintln!("  Please manually delete '{}'.", to_utf(&output_path));
-                    eprintln!(
-                        "  Compression failed and we could not delete '{}'.",
-                        to_utf(&output_path),
-                    );
-                    eprintln!(
-                        "  Error:{reset} {}{red}.{reset}\n",
-                        err,
-                        reset = *colors::RESET,
-                        red = *colors::RED
-                    );
+                    eprintln!("  Ouch failed to delete the file '{}'.", to_utf(&output_path));
+                    eprintln!("  Please delete it manually.");
+                    eprintln!("  This file is corrupted if compression didn't finished.");
+
+                    if compress_result.is_err() {
+                        eprintln!("  Compression failed for reasons below.");
+                    }
                 }
             }
 

src/utils/fs.rs

@@ -29,13 +29,18 @@ pub fn clear_path(path: &Path, question_policy: QuestionPolicy) -> crate::Result
         return Ok(false);
     }
 
+    remove_file_or_dir(path)?;
+
+    Ok(true)
+}
+
+pub fn remove_file_or_dir(path: &Path) -> crate::Result<()> {
     if path.is_dir() {
         fs::remove_dir_all(path)?;
     } else if path.is_file() {
         fs::remove_file(path)?;
     }
-
-    Ok(true)
+    Ok(())
 }
 
 /// Creates a directory at the path, if there is nothing there.

src/utils/mod.rs

@@ -12,7 +12,8 @@ mod question;
 pub use file_visibility::FileVisibilityPolicy;
 pub use formatting::{nice_directory_display, pretty_format_list_of_paths, strip_cur_dir, to_utf};
 pub use fs::{
-    cd_into_same_dir_as, clear_path, create_dir_if_non_existent, dir_is_empty, is_symlink, try_infer_extension,
+    cd_into_same_dir_as, clear_path, create_dir_if_non_existent, dir_is_empty, is_symlink, remove_file_or_dir,
+    try_infer_extension,
 };
 pub use question::{
     ask_to_create_file, user_wants_to_continue, user_wants_to_overwrite, QuestionAction, QuestionPolicy,

src/utils/question.rs

@@ -15,7 +15,7 @@ use super::{strip_cur_dir, to_utf};
 use crate::{
     accessible::is_running_in_accessible_mode,
     error::{Error, Result},
-    utils::colors,
+    utils::{self, colors},
 };
 
 #[derive(Debug, PartialEq, Eq, Clone, Copy)]
@@ -59,9 +59,7 @@ pub fn ask_to_create_file(path: &Path, question_policy: QuestionPolicy) -> Resul
         Ok(w) => Ok(Some(w)),
         Err(e) if e.kind() == std::io::ErrorKind::AlreadyExists => {
             if user_wants_to_overwrite(path, question_policy)? {
-                if path.is_dir() {
-                    fs::remove_dir_all(path)?;
-                }
+                utils::remove_file_or_dir(path)?;
                 Ok(Some(fs::File::create(path)?))
             } else {
                 Ok(None)