diff --git a/src/check.rs b/src/check.rs index 9e1bdac37..afb3c3b01 100644 --- a/src/check.rs +++ b/src/check.rs @@ -10,7 +10,7 @@ use std::{ use crate::{ error::FinalError, - extension::{build_archive_file_suggestion, Extension, PRETTY_SUPPORTED_ALIASES, PRETTY_SUPPORTED_EXTENSIONS}, + extension::{build_archive_file_suggestion, Extension}, info, utils::{pretty_format_list_of_paths, try_infer_extension, user_wants_to_continue, EscapedPathDisplay}, warning, QuestionAction, QuestionPolicy, Result, @@ -159,10 +159,8 @@ pub fn check_missing_formats_when_decompressing(files: &[PathBuf], formats: &[Ve )); } - error = error - .detail("Decompression formats are detected automatically from file extension") - .hint(format!("Supported extensions are: {}", PRETTY_SUPPORTED_EXTENSIONS)) - .hint(format!("Supported aliases are: {}", PRETTY_SUPPORTED_ALIASES)); + error = error.detail("Decompression formats are detected automatically from file extension"); + error = error.hint_all_supported_formats(); // If there's exactly one file, give a suggestion to use `--format` if let &[path] = files_with_broken_extension.as_slice() { diff --git a/src/commands/mod.rs b/src/commands/mod.rs index 2d741406c..d8c0709ba 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -14,10 +14,10 @@ use crate::{ cli::Subcommand, commands::{compress::compress_files, decompress::decompress_file, list::list_archive_contents}, error::{Error, FinalError}, - extension::{self, parse_format}, + extension::{self, parse_format_flag}, info, list::ListOptions, - utils::{self, to_utf, EscapedPathDisplay, FileVisibilityPolicy}, + utils::{self, path_to_str, EscapedPathDisplay, FileVisibilityPolicy}, warning, CliArgs, QuestionPolicy, }; @@ -56,7 +56,7 @@ pub fn run( // Formats from path extension, like "file.tar.gz.xz" -> vec![Tar, Gzip, Lzma] let (formats_from_flag, formats) = match args.format { Some(formats) => { - let parsed_formats = parse_format(&formats)?; + let parsed_formats = parse_format_flag(&formats)?; (Some(formats), parsed_formats) } None => (None, extension::extensions_from_path(&output_path)), @@ -99,7 +99,7 @@ pub fn run( // having a final status message is important especially in an accessibility context // as screen readers may not read a commands exit code, making it hard to reason // about whether the command succeeded without such a message - info!(accessible, "Successfully compressed '{}'.", to_utf(&output_path)); + info!(accessible, "Successfully compressed '{}'.", path_to_str(&output_path)); } else { // If Ok(false) or Err() occurred, delete incomplete file at `output_path` // @@ -127,7 +127,7 @@ pub fn run( let mut formats = vec![]; if let Some(format) = args.format { - let format = parse_format(&format)?; + let format = parse_format_flag(&format)?; for path in files.iter() { let file_name = path.file_name().ok_or_else(|| Error::NotFound { error_title: format!("{} does not have a file name", EscapedPathDisplay::new(path)), @@ -179,7 +179,7 @@ pub fn run( let mut formats = vec![]; if let Some(format) = args.format { - let format = parse_format(&format)?; + let format = parse_format_flag(&format)?; for _ in 0..files.len() { formats.push(format.clone()); } diff --git a/src/error.rs b/src/error.rs index a8832ea79..481d541e4 100644 --- a/src/error.rs +++ b/src/error.rs @@ -4,13 +4,18 @@ use std::{ borrow::Cow, + ffi::OsString, fmt::{self, Display}, }; -use crate::{accessible::is_running_in_accessible_mode, utils::colors::*}; +use crate::{ + accessible::is_running_in_accessible_mode, + extension::{PRETTY_SUPPORTED_ALIASES, PRETTY_SUPPORTED_EXTENSIONS}, + utils::os_str_to_str, +}; /// All errors that can be generated by `ouch` -#[derive(Debug)] +#[derive(Debug, Clone)] pub enum Error { /// Not every IoError, some of them get filtered by `From` into other variants IoError { reason: String }, @@ -33,7 +38,7 @@ pub enum Error { /// Custom and unique errors are reported in this variant Custom { reason: FinalError }, /// Invalid format passed to `--format` - InvalidFormat { reason: String }, + InvalidFormatFlag { text: OsString, reason: String }, } /// Alias to std's Result with ouch's Error @@ -55,6 +60,8 @@ pub struct FinalError { impl Display for FinalError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + use crate::utils::colors::*; + // Title // // When in ACCESSIBLE mode, the square brackets are suppressed @@ -115,32 +122,54 @@ impl FinalError { self.hints.push(hint.into()); self } + + /// Adds all supported formats as hints. + /// + /// This is what it looks like: + /// ``` + /// hint: Supported extensions are: tar, zip, bz, bz2, gz, lz4, xz, lzma, sz, zst + /// hint: Supported aliases are: tgz, tbz, tlz4, txz, tzlma, tsz, tzst + /// ``` + pub fn hint_all_supported_formats(self) -> Self { + self.hint(format!("Supported extensions are: {}", PRETTY_SUPPORTED_EXTENSIONS)) + .hint(format!("Supported aliases are: {}", PRETTY_SUPPORTED_ALIASES)) + } } -impl fmt::Display for Error { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - let err = match self { - Error::WalkdirError { reason } => FinalError::with_title(reason.to_string()), - Error::NotFound { error_title } => FinalError::with_title(error_title.to_string()).detail("File not found"), +impl From<&Error> for FinalError { + fn from(err: &Error) -> Self { + match err.clone() { + Error::WalkdirError { reason } => FinalError::with_title(reason), + Error::NotFound { error_title } => FinalError::with_title(error_title).detail("File not found"), Error::CompressingRootFolder => { FinalError::with_title("It seems you're trying to compress the root folder.") .detail("This is unadvisable since ouch does compressions in-memory.") .hint("Use a more appropriate tool for this, such as rsync.") } - Error::IoError { reason } => FinalError::with_title(reason.to_string()), - Error::Lz4Error { reason } => FinalError::with_title(reason.to_string()), - Error::AlreadyExists { error_title } => { - FinalError::with_title(error_title.to_string()).detail("File already exists") + Error::IoError { reason } => FinalError::with_title(reason), + Error::Lz4Error { reason } => FinalError::with_title(reason), + Error::AlreadyExists { error_title } => FinalError::with_title(error_title).detail("File already exists"), + Error::InvalidZipArchive(reason) => FinalError::with_title("Invalid zip archive").detail(reason), + Error::PermissionDenied { error_title } => FinalError::with_title(error_title).detail("Permission denied"), + Error::UnsupportedZipArchive(reason) => FinalError::with_title("Unsupported zip archive").detail(reason), + Error::InvalidFormatFlag { reason, text } => { + FinalError::with_title(format!("Failed to parse `--format {}`", os_str_to_str(&text))) + .detail(reason) + .hint_all_supported_formats() + .hint("") + .hint("Examples:") + .hint(" --format tar") + .hint(" --format gz") + .hint(" --format tar.gz") } - Error::InvalidZipArchive(reason) => FinalError::with_title("Invalid zip archive").detail(*reason), - Error::PermissionDenied { error_title } => { - FinalError::with_title(error_title.to_string()).detail("Permission denied") - } - Error::UnsupportedZipArchive(reason) => FinalError::with_title("Unsupported zip archive").detail(*reason), - Error::InvalidFormat { reason } => FinalError::with_title("Invalid archive format").detail(reason.clone()), - Error::Custom { reason } => reason.clone(), - }; + Error::Custom { reason } => reason, + } + } +} +impl fmt::Display for Error { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + let err = FinalError::from(self); write!(f, "{err}") } } diff --git a/src/extension.rs b/src/extension.rs index 2d3f789f1..b216812dc 100644 --- a/src/extension.rs +++ b/src/extension.rs @@ -3,8 +3,8 @@ use std::{ffi::OsStr, fmt, path::Path}; use bstr::ByteSlice; +use CompressionFormat::*; -use self::CompressionFormat::*; use crate::{error::Error, warning}; pub const SUPPORTED_EXTENSIONS: &[&str] = &["tar", "zip", "bz", "bz2", "gz", "lz4", "xz", "lzma", "sz", "zst"]; @@ -14,7 +14,10 @@ pub const PRETTY_SUPPORTED_ALIASES: &str = "tgz, tbz, tlz4, txz, tzlma, tsz, tzs /// A wrapper around `CompressionFormat` that allows combinations like `tgz` #[derive(Debug, Clone)] +// Keep `PartialEq` only for testing because two formats are the same even if +// their `display_text` does not match (beware of aliases) #[cfg_attr(test, derive(PartialEq))] +// Should only be built with constructors #[non_exhaustive] pub struct Extension { /// One extension like "tgz" can be made of multiple CompressionFormats ([Tar, Gz]) @@ -117,17 +120,30 @@ fn split_extension(name: &mut &[u8]) -> Option { Some(ext) } -pub fn parse_format(fmt: &OsStr) -> crate::Result> { - let fmt = <[u8] as ByteSlice>::from_os_str(fmt).ok_or_else(|| Error::InvalidFormat { - reason: "Invalid UTF-8".into(), +pub fn parse_format_flag(input: &OsStr) -> crate::Result> { + let format = input.as_encoded_bytes(); + + let format = std::str::from_utf8(format).map_err(|_| Error::InvalidFormatFlag { + text: input.to_owned(), + reason: "Invalid UTF-8.".to_string(), })?; - let mut extensions = Vec::new(); - for extension in fmt.split_str(b".") { - let extension = to_extension(extension).ok_or_else(|| Error::InvalidFormat { - reason: format!("Unsupported extension: {}", extension.to_str_lossy()), - })?; - extensions.push(extension); + let extensions: Vec = format + .split('.') + .filter(|extension| !extension.is_empty()) + .map(|extension| { + to_extension(extension.as_bytes()).ok_or_else(|| Error::InvalidFormatFlag { + text: input.to_owned(), + reason: format!("Unsupported extension '{}'", extension), + }) + }) + .collect::>()?; + + if extensions.is_empty() { + return Err(Error::InvalidFormatFlag { + text: input.to_owned(), + reason: "Parsing got an empty list of extensions.".to_string(), + }); } Ok(extensions) @@ -230,6 +246,7 @@ mod tests { } #[test] + /// Test extension parsing for input/output files fn test_separate_known_extensions_from_name() { assert_eq!( separate_known_extensions_from_name("file".as_ref()), @@ -260,6 +277,37 @@ mod tests { ); } + #[test] + /// Test extension parsing of `--format FORMAT` + fn test_parse_of_format_flag() { + assert_eq!( + parse_format_flag(OsStr::new("tar")).unwrap(), + vec![Extension::new(&[Tar], "tar")] + ); + assert_eq!( + parse_format_flag(OsStr::new(".tar")).unwrap(), + vec![Extension::new(&[Tar], "tar")] + ); + assert_eq!( + parse_format_flag(OsStr::new("tar.gz")).unwrap(), + vec![Extension::new(&[Tar], "tar"), Extension::new(&[Gzip], "gz")] + ); + assert_eq!( + parse_format_flag(OsStr::new(".tar.gz")).unwrap(), + vec![Extension::new(&[Tar], "tar"), Extension::new(&[Gzip], "gz")] + ); + assert_eq!( + parse_format_flag(OsStr::new("..tar..gz.....")).unwrap(), + vec![Extension::new(&[Tar], "tar"), Extension::new(&[Gzip], "gz")] + ); + + assert!(parse_format_flag(OsStr::new("../tar.gz")).is_err()); + assert!(parse_format_flag(OsStr::new("targz")).is_err()); + assert!(parse_format_flag(OsStr::new("tar.gz.unknown")).is_err()); + assert!(parse_format_flag(OsStr::new(".tar.gz.unknown")).is_err()); + assert!(parse_format_flag(OsStr::new(".tar.!@#.gz")).is_err()); + } + #[test] fn builds_suggestion_correctly() { assert_eq!(build_archive_file_suggestion(Path::new("linux.png"), ".tar"), None); diff --git a/src/utils/formatting.rs b/src/utils/formatting.rs index fac5c1126..9ebef9698 100644 --- a/src/utils/formatting.rs +++ b/src/utils/formatting.rs @@ -1,4 +1,4 @@ -use std::{borrow::Cow, cmp, fmt::Display, path::Path}; +use std::{borrow::Cow, cmp, ffi::OsStr, fmt::Display, path::Path}; use crate::CURRENT_DIRECTORY; @@ -45,7 +45,11 @@ impl Display for EscapedPathDisplay<'_> { /// This is different from [`Path::display`]. /// /// See for a comparison. -pub fn to_utf(os_str: &Path) -> Cow { +pub fn path_to_str(path: &Path) -> Cow { + os_str_to_str(path.as_ref()) +} + +pub fn os_str_to_str(os_str: &OsStr) -> Cow { let format = || { let text = format!("{os_str:?}"); Cow::Owned(text.trim_matches('"').to_string()) @@ -65,15 +69,15 @@ pub fn strip_cur_dir(source_path: &Path) -> &Path { /// Converts a slice of `AsRef` to comma separated String /// /// Panics if the slice is empty. -pub fn pretty_format_list_of_paths(os_strs: &[impl AsRef]) -> String { - let mut iter = os_strs.iter().map(AsRef::as_ref); +pub fn pretty_format_list_of_paths(paths: &[impl AsRef]) -> String { + let mut iter = paths.iter().map(AsRef::as_ref); - let first_element = iter.next().unwrap(); - let mut string = to_utf(first_element).into_owned(); + let first_path = iter.next().unwrap(); + let mut string = path_to_str(first_path).into_owned(); - for os_str in iter { + for path in iter { string += ", "; - string += &to_utf(os_str); + string += &path_to_str(path); } string } @@ -83,7 +87,7 @@ pub fn nice_directory_display(path: &Path) -> Cow { if path == Path::new(".") { Cow::Borrowed("current directory") } else { - to_utf(path) + path_to_str(path) } } diff --git a/src/utils/mod.rs b/src/utils/mod.rs index b6ea08e50..a27d6295d 100644 --- a/src/utils/mod.rs +++ b/src/utils/mod.rs @@ -9,17 +9,19 @@ mod formatting; mod fs; mod question; -pub use file_visibility::FileVisibilityPolicy; -pub use formatting::{ - nice_directory_display, pretty_format_list_of_paths, strip_cur_dir, to_utf, Bytes, EscapedPathDisplay, +pub use self::{ + file_visibility::FileVisibilityPolicy, + formatting::{ + nice_directory_display, os_str_to_str, path_to_str, pretty_format_list_of_paths, strip_cur_dir, Bytes, + EscapedPathDisplay, + }, + fs::{ + cd_into_same_dir_as, clear_path, create_dir_if_non_existent, is_symlink, remove_file_or_dir, + try_infer_extension, + }, + question::{ask_to_create_file, user_wants_to_continue, user_wants_to_overwrite, QuestionAction, QuestionPolicy}, + utf8::{get_invalid_utf8_paths, is_invalid_utf8}, }; -pub use fs::{ - 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, -}; -pub use utf8::{get_invalid_utf8_paths, is_invalid_utf8}; mod utf8 { use std::{ffi::OsStr, path::PathBuf}; diff --git a/src/utils/question.rs b/src/utils/question.rs index 713931e39..92e599f65 100644 --- a/src/utils/question.rs +++ b/src/utils/question.rs @@ -11,11 +11,10 @@ use std::{ use fs_err as fs; -use super::{strip_cur_dir, to_utf}; use crate::{ accessible::is_running_in_accessible_mode, error::{Error, FinalError, Result}, - utils::{self, colors}, + utils::{self, colors, formatting::path_to_str, strip_cur_dir}, }; #[derive(Debug, PartialEq, Eq, Clone, Copy)] @@ -44,7 +43,7 @@ pub fn user_wants_to_overwrite(path: &Path, question_policy: QuestionPolicy) -> QuestionPolicy::AlwaysYes => Ok(true), QuestionPolicy::AlwaysNo => Ok(false), QuestionPolicy::Ask => { - let path = to_utf(strip_cur_dir(path)); + let path = path_to_str(strip_cur_dir(path)); let path = Some(&*path); let placeholder = Some("FILE"); Confirmation::new("Do you want to overwrite 'FILE'?", placeholder).ask(path) @@ -83,7 +82,7 @@ pub fn user_wants_to_continue( QuestionAction::Compression => "compress", QuestionAction::Decompression => "decompress", }; - let path = to_utf(strip_cur_dir(path)); + let path = path_to_str(strip_cur_dir(path)); let path = Some(&*path); let placeholder = Some("FILE"); Confirmation::new(&format!("Do you want to {action} 'FILE'?"), placeholder).ask(path) diff --git a/tests/snapshots/ui__ui_test_err_format_flag-2.snap b/tests/snapshots/ui__ui_test_err_format_flag-2.snap new file mode 100644 index 000000000..a552d96f6 --- /dev/null +++ b/tests/snapshots/ui__ui_test_err_format_flag-2.snap @@ -0,0 +1,15 @@ +--- +source: tests/ui.rs +expression: "run_ouch(\"ouch compress input output --format targz\", dir)" +--- +[ERROR] Failed to parse `--format targz` + - Unsupported extension 'targz' + +hint: Supported extensions are: tar, zip, bz, bz2, gz, lz4, xz, lzma, sz, zst +hint: Supported aliases are: tgz, tbz, tlz4, txz, tzlma, tsz, tzst +hint: +hint: Examples: +hint: --format tar +hint: --format gz +hint: --format tar.gz + diff --git a/tests/snapshots/ui__ui_test_err_format_flag-3.snap b/tests/snapshots/ui__ui_test_err_format_flag-3.snap new file mode 100644 index 000000000..940f93a92 --- /dev/null +++ b/tests/snapshots/ui__ui_test_err_format_flag-3.snap @@ -0,0 +1,15 @@ +--- +source: tests/ui.rs +expression: "run_ouch(\"ouch compress input output --format .tar.$#!@.rest\", dir)" +--- +[ERROR] Failed to parse `--format .tar.$#!@.rest` + - Unsupported extension '$#!@' + +hint: Supported extensions are: tar, zip, bz, bz2, gz, lz4, xz, lzma, sz, zst +hint: Supported aliases are: tgz, tbz, tlz4, txz, tzlma, tsz, tzst +hint: +hint: Examples: +hint: --format tar +hint: --format gz +hint: --format tar.gz + diff --git a/tests/snapshots/ui__ui_test_err_format_flag.snap b/tests/snapshots/ui__ui_test_err_format_flag.snap new file mode 100644 index 000000000..ff1860d7e --- /dev/null +++ b/tests/snapshots/ui__ui_test_err_format_flag.snap @@ -0,0 +1,15 @@ +--- +source: tests/ui.rs +expression: "run_ouch(\"ouch compress input output --format tar.gz.unknown\", dir)" +--- +[ERROR] Failed to parse `--format tar.gz.unknown` + - Unsupported extension 'unknown' + +hint: Supported extensions are: tar, zip, bz, bz2, gz, lz4, xz, lzma, sz, zst +hint: Supported aliases are: tgz, tbz, tlz4, txz, tzlma, tsz, tzst +hint: +hint: Examples: +hint: --format tar +hint: --format gz +hint: --format tar.gz + diff --git a/tests/snapshots/ui__ui_test_ok_format_flag-2.snap b/tests/snapshots/ui__ui_test_ok_format_flag-2.snap new file mode 100644 index 000000000..ac57ca5eb --- /dev/null +++ b/tests/snapshots/ui__ui_test_ok_format_flag-2.snap @@ -0,0 +1,7 @@ +--- +source: tests/ui.rs +expression: "run_ouch(\"ouch compress input output2 --format .tar.gz\", dir)" +--- +[INFO] Compressing 'input'. +[INFO] Successfully compressed 'output2'. + diff --git a/tests/snapshots/ui__ui_test_ok_format_flag.snap b/tests/snapshots/ui__ui_test_ok_format_flag.snap new file mode 100644 index 000000000..f4f38c59d --- /dev/null +++ b/tests/snapshots/ui__ui_test_ok_format_flag.snap @@ -0,0 +1,7 @@ +--- +source: tests/ui.rs +expression: "run_ouch(\"ouch compress input output1 --format tar.gz\", dir)" +--- +[INFO] Compressing 'input'. +[INFO] Successfully compressed 'output1'. + diff --git a/tests/ui.rs b/tests/ui.rs index c72f05224..fba46daa9 100644 --- a/tests/ui.rs +++ b/tests/ui.rs @@ -87,6 +87,29 @@ fn ui_test_err_missing_files() { ui!(run_ouch("ouch list a b", dir)); } +#[test] +fn ui_test_err_format_flag() { + let (_dropper, dir) = testdir().unwrap(); + + // prepare + run_in(dir, "touch", "input").unwrap(); + + ui!(run_ouch("ouch compress input output --format tar.gz.unknown", dir)); + ui!(run_ouch("ouch compress input output --format targz", dir)); + ui!(run_ouch("ouch compress input output --format .tar.$#!@.rest", dir)); +} + +#[test] +fn ui_test_ok_format_flag() { + let (_dropper, dir) = testdir().unwrap(); + + // prepare + run_in(dir, "touch", "input").unwrap(); + + ui!(run_ouch("ouch compress input output1 --format tar.gz", dir)); + ui!(run_ouch("ouch compress input output2 --format .tar.gz", dir)); +} + #[test] fn ui_test_ok_compress() { let (_dropper, dir) = testdir().unwrap(); diff --git a/tests/utils.rs b/tests/utils.rs index 3682e99bf..ba9f1d7e5 100644 --- a/tests/utils.rs +++ b/tests/utils.rs @@ -50,8 +50,9 @@ pub fn run_in(folder: impl AsRef, bin: impl AsRef, args: &str) -> i // write random content to a file pub fn write_random_content(file: &mut impl Write, rng: &mut impl RngCore) { - let mut data = Vec::new(); - data.resize(rng.gen_range(0..4096), 0); + let len = rng.gen_range(0..4096); + let mut data = vec![0; len]; + rng.fill_bytes(&mut data); file.write_all(&data).unwrap(); }