From e8d0914a01659e59789f0c7f3b0315970c7b3661 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jo=C3=A3o=20M=2E=20Bezerra?= <marcospb19@hotmail.com>
Date: Mon, 17 Oct 2022 00:03:44 -0300
Subject: [PATCH] improve error message when compressing folder with
 single-file formats

---
 src/commands/mod.rs | 36 +++++++++++++++++-------------------
 src/utils/fs.rs     |  8 --------
 src/utils/mod.rs    |  3 +--
 3 files changed, 18 insertions(+), 29 deletions(-)

diff --git a/src/commands/mod.rs b/src/commands/mod.rs
index cbab242..32ff508 100644
--- a/src/commands/mod.rs
+++ b/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,
-        FileVisibilityPolicy,
+        self, pretty_format_list_of_paths, to_utf, try_infer_extension, user_wants_to_continue, 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
 ///
@@ -116,23 +105,32 @@ pub fn run(
                 return Err(error.into());
             }
 
-            if !formats.get(0).map(Extension::is_archive).unwrap_or(false) && represents_several_files(&files) {
+            let is_some_input_a_folder = files.iter().any(|path| path.is_dir());
+            let is_multiple_inputs = files.len() > 1;
+
+            // If first format is not archive, can't compress folder, or multiple files
+            // Index safety: empty formats should be checked above.
+            if !formats[0].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 +270,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();
 
diff --git a/src/utils/fs.rs b/src/utils/fs.rs
index 4779549..d9f577c 100644
--- a/src/utils/fs.rs
+++ b/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,
diff --git a/src/utils/mod.rs b/src/utils/mod.rs
index e2c3130..61a65bd 100644
--- a/src/utils/mod.rs
+++ b/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,
-    try_infer_extension,
+    cd_into_same_dir_as, clear_path, create_dir_if_non_existent, 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,