Merge pull request #303 from ouch-org/improve-error-message-when-compressing-folder-with-single-file-format

Improve error message when compressing folder with single-file formats

CHANGELOG.md

@@ -79,6 +79,7 @@ Categories Used:
 - Check for errors when setting the last modified time [\#278](https://github.com/ouch-org/ouch/pull/278) ([marcospb19](https://github.com/marcospb19))
 - Use to the humansize crate for formatting human-readable file sizes [\#281](https://github.com/ouch-org/ouch/pull/281) ([figsoda](https://github.com/figsoda))
 - Reactivate CI targets for ARM Linux and Windows MinGW [\#289](https://github.com/ouch-org/ouch/pull/289) ([figsoda](https://github.com/figsoda))
+- Improve error message when compressing folder with single-file formats [\#303](https://github.com/ouch-org/ouch/pull/303) ([marcospb19](https://github.com/marcospb19))
 
 ### Tweaks
 

src/commands/mod.rs

@@ -18,8 +18,7 @@ use crate::{
     info,
     list::ListOptions,
     utils::{
-        self, dir_is_empty, pretty_format_list_of_paths, to_utf, try_infer_extension, user_wants_to_continue,
+        self, pretty_format_list_of_paths, to_utf, try_infer_extension, user_wants_to_continue, FileVisibilityPolicy,
-        FileVisibilityPolicy,
     },
     warning, Opts, QuestionAction, QuestionPolicy, Subcommand,
 };
@@ -34,16 +33,6 @@ fn warn_user_about_loading_zip_in_memory() {
     warning!("{}", ZIP_IN_MEMORY_LIMITATION_WARNING);
 }
 
-fn represents_several_files(files: &[PathBuf]) -> bool {
-    let is_non_empty_dir = |path: &PathBuf| {
-        let is_non_empty = || !dir_is_empty(path);
-
-        path.is_dir().then(is_non_empty).unwrap_or_default()
-    };
-
-    files.iter().any(is_non_empty_dir) || files.len() > 1
-}
-
 /// Builds a suggested output file in scenarios where the user tried to compress
 /// a folder into a non-archive compression format, for error message purposes
 ///
@@ -100,39 +89,44 @@ pub fn run(
             // Formats from path extension, like "file.tar.gz.xz" -> vec![Tar, Gzip, Lzma]
             let formats = extension::extensions_from_path(&output_path);
 
-            if formats.is_empty() {
+            let first_format = formats.first().ok_or_else(|| {
-                let error = FinalError::with_title(format!("Cannot compress to '{}'.", to_utf(&output_path)))
+                let output_path = to_utf(&output_path);
+                FinalError::with_title(format!("Cannot compress to '{output_path}'."))
                     .detail("You shall supply the compression format")
                     .hint("Try adding supported extensions (see --help):")
-                    .hint(format!("  ouch compress <FILES>... {}.tar.gz", to_utf(&output_path)))
+                    .hint(format!("  ouch compress <FILES>... {output_path}.tar.gz"))
-                    .hint(format!("  ouch compress <FILES>... {}.zip", to_utf(&output_path)))
+                    .hint(format!("  ouch compress <FILES>... {output_path}.zip"))
                     .hint("")
                     .hint("Alternatively, you can overwrite this option by using the '--format' flag:")
-                    .hint(format!(
+                    .hint(format!("  ouch compress <FILES>... {output_path} --format tar.gz"))
-                        "  ouch compress <FILES>... {} --format tar.gz",
+            })?;
-                        to_utf(&output_path)
-                    ));
 
-                return Err(error.into());
+            let is_some_input_a_folder = files.iter().any(|path| path.is_dir());
-            }
+            let is_multiple_inputs = files.len() > 1;
 
-            if !formats.get(0).map(Extension::is_archive).unwrap_or(false) && represents_several_files(&files) {
+            // If first format is not archive, can't compress folder, or multiple files
+            // Index safety: empty formats should be checked above.
+            if !first_format.is_archive() && (is_some_input_a_folder || is_multiple_inputs) {
                 // This piece of code creates a suggestion for compressing multiple files
                 // It says:
                 // Change from file.bz.xz
                 // To          file.tar.bz.xz
                 let suggested_output_path = build_archive_file_suggestion(&output_path, ".tar")
-                    .expect("output path did not contain a compression format");
+                    .expect("output path should contain a compression format");
-
                 let output_path = to_utf(&output_path);
+                let first_detail_message = if is_multiple_inputs {
+                    "You are trying to compress multiple files."
+                } else {
+                    "You are trying to compress a folder."
+                };
 
                 let error = FinalError::with_title(format!("Cannot compress to '{}'.", output_path))
-                    .detail("You are trying to compress multiple files.")
+                    .detail(first_detail_message)
                     .detail(format!(
-                        "The compression format '{}' cannot receive multiple files.",
+                        "The compression format '{}' does not accept multiple files.",
                         &formats[0]
                     ))
-                    .detail("The only supported formats that archive files into an archive are .tar and .zip.")
+                    .detail("Formats that bundle files into an archive are .tar and .zip.")
                     .hint(format!("Try inserting '.tar' or '.zip' before '{}'.", &formats[0]))
                     .hint(format!("From: {}", output_path))
                     .hint(format!("To:   {}", suggested_output_path));
@@ -272,7 +266,7 @@ pub fn run(
             let not_archives: Vec<PathBuf> = files
                 .iter()
                 .zip(&formats)
-                .filter(|(_, formats)| !formats.get(0).map(Extension::is_archive).unwrap_or(false))
+                .filter(|(_, formats)| !formats.first().map(Extension::is_archive).unwrap_or(false))
                 .map(|(path, _)| path.clone())
                 .collect();
 

src/utils/fs.rs

@@ -2,7 +2,6 @@
 
 use std::{
     env,
-    fs::ReadDir,
     io::Read,
     path::{Path, PathBuf},
 };
@@ -12,13 +11,6 @@ use fs_err as fs;
 use super::{to_utf, user_wants_to_overwrite};
 use crate::{extension::Extension, info, QuestionPolicy};
 
-/// Checks if given path points to an empty directory.
-pub fn dir_is_empty(dir_path: &Path) -> bool {
-    let is_empty = |mut rd: ReadDir| rd.next().is_none();
-
-    dir_path.read_dir().map(is_empty).unwrap_or_default()
-}
-
 /// Remove `path` asking the user to overwrite if necessary.
 ///
 /// * `Ok(true)` means the path is clear,

src/utils/mod.rs

@@ -12,8 +12,7 @@ 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, remove_file_or_dir,
+    cd_into_same_dir_as, clear_path, create_dir_if_non_existent, is_symlink, remove_file_or_dir, try_infer_extension,
-    try_infer_extension,
 };
 pub use question::{
     ask_to_create_file, user_wants_to_continue, user_wants_to_overwrite, QuestionAction, QuestionPolicy,