diff --git a/CHANGELOG.md b/CHANGELOG.md index 56677a0c0..3b9e38e0a 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/src/commands/mod.rs b/src/commands/mod.rs index cbab24222..a82f4d8ad 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 /// @@ -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 error = FinalError::with_title(format!("Cannot compress to '{}'.", to_utf(&output_path))) + let first_format = formats.first().ok_or_else(|| { + 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 ... {}.tar.gz", to_utf(&output_path))) - .hint(format!(" ouch compress ... {}.zip", to_utf(&output_path))) + .hint(format!(" ouch compress ... {output_path}.tar.gz")) + .hint(format!(" ouch compress ... {output_path}.zip")) .hint("") .hint("Alternatively, you can overwrite this option by using the '--format' flag:") - .hint(format!( - " ouch compress ... {} --format tar.gz", - to_utf(&output_path) - )); + .hint(format!(" ouch compress ... {output_path} --format tar.gz")) + })?; - 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 = 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 477954902..d9f577ce5 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 e2c313008..61a65bd88 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,