Fix --format parsing extensions with dots

Also improve error reporting for `--format` with malformed or
unsupported extensions

This commit is very messy, as it also does an refac in the project,
which should ideally be in a separated commit
This commit is contained in:
João M. Bezerra 2023-09-18 00:07:07 -03:00 committed by João Marcos
parent 1a80b919e3
commit 5dac8431f2
16 changed files with 250 additions and 81 deletions

View File

@ -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},
utils::{
logger::{info_accessible, warning},
pretty_format_list_of_paths, try_infer_extension, user_wants_to_continue, EscapedPathDisplay,
@ -160,10 +160,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() {

View File

@ -15,10 +15,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},
list::ListOptions,
utils::{
self, colors::*, is_path_stdin, logger::info_accessible, to_utf, EscapedPathDisplay, FileVisibilityPolicy,
self, colors::*, is_path_stdin, logger::info_accessible, path_to_str, EscapedPathDisplay, FileVisibilityPolicy,
},
CliArgs, QuestionPolicy,
};
@ -68,7 +68,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)),
@ -111,7 +111,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(format!("Successfully compressed '{}'.", to_utf(&output_path)));
info_accessible(format!("Successfully compressed '{}'.", path_to_str(&output_path)));
} else {
// If Ok(false) or Err() occurred, delete incomplete file at `output_path`
//
@ -139,7 +139,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)),
@ -199,7 +199,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());
}

View File

@ -4,15 +4,21 @@
use std::{
borrow::Cow,
ffi::OsString,
fmt::{self, Display},
io,
};
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<io::Error>` into other variants
/// An IoError that doesn't have a dedicated error variant
IoError { reason: String },
/// From lzzzz::lz4f::Error
Lz4Error { reason: String },
@ -33,9 +39,9 @@ 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 },
/// From sevenz_rust::Error
SevenzipError(sevenz_rust::Error),
SevenzipError { reason: String },
/// Recognised but unsupported format
// currently only RAR when built without the `unrar` feature
UnsupportedFormat { reason: String },
@ -62,6 +68,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
@ -122,56 +130,72 @@ 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 {
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::SevenzipError(reason) => FinalError::with_title("7z error").detail(reason.to_string()),
Error::SevenzipError { reason } => FinalError::with_title("7z error").detail(reason),
Error::UnsupportedFormat { reason } => {
FinalError::with_title("Recognised but unsupported format").detail(reason.clone())
}
Error::InvalidPassword { reason } => FinalError::with_title("Invalid password").detail(reason.clone()),
};
}
}
}
impl fmt::Display for Error {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
let err = FinalError::from(self.clone());
write!(f, "{err}")
}
}
impl From<std::io::Error> for Error {
fn from(err: std::io::Error) -> Self {
let error_title = err.to_string();
match err.kind() {
std::io::ErrorKind::NotFound => Self::NotFound {
error_title: err.to_string(),
},
std::io::ErrorKind::PermissionDenied => Self::PermissionDenied {
error_title: err.to_string(),
},
std::io::ErrorKind::AlreadyExists => Self::AlreadyExists {
error_title: err.to_string(),
},
_other => Self::IoError {
reason: err.to_string(),
},
io::ErrorKind::NotFound => Self::NotFound { error_title },
io::ErrorKind::PermissionDenied => Self::PermissionDenied { error_title },
io::ErrorKind::AlreadyExists => Self::AlreadyExists { error_title },
_other => Self::IoError { reason: error_title },
}
}
}
@ -201,7 +225,9 @@ impl From<unrar::error::UnrarError> for Error {
impl From<sevenz_rust::Error> for Error {
fn from(err: sevenz_rust::Error) -> Self {
Self::SevenzipError(err)
Self::SevenzipError {
reason: err.to_string(),
}
}
}

View File

@ -3,8 +3,8 @@
use std::{ffi::OsStr, fmt, path::Path};
use bstr::ByteSlice;
use CompressionFormat::*;
use self::CompressionFormat::*;
use crate::{error::Error, utils::logger::warning};
pub const SUPPORTED_EXTENSIONS: &[&str] = &[
@ -34,7 +34,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])
@ -144,17 +147,30 @@ fn split_extension(name: &mut &[u8]) -> Option<Extension> {
Some(ext)
}
pub fn parse_format(fmt: &OsStr) -> crate::Result<Vec<Extension>> {
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<Vec<Extension>> {
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<Extension> = 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::<crate::Result<_>>()?;
if extensions.is_empty() {
return Err(Error::InvalidFormatFlag {
text: input.to_owned(),
reason: "Parsing got an empty list of extensions.".to_string(),
});
}
Ok(extensions)
@ -245,6 +261,7 @@ pub fn build_archive_file_suggestion(path: &Path, suggested_extension: &str) ->
#[cfg(test)]
mod tests {
use super::*;
use crate::utils::logger::spawn_logger_thread;
#[test]
fn test_extensions_from_path() {
@ -257,7 +274,9 @@ mod tests {
}
#[test]
/// Test extension parsing for input/output files
fn test_separate_known_extensions_from_name() {
let _handler = spawn_logger_thread();
assert_eq!(
separate_known_extensions_from_name("file".as_ref()),
("file".as_ref(), vec![])
@ -287,6 +306,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);

View File

@ -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 <https://gist.github.com/marcospb19/ebce5572be26397cf08bbd0fd3b65ac1> for a comparison.
pub fn to_utf(os_str: &Path) -> Cow<str> {
pub fn path_to_str(path: &Path) -> Cow<str> {
os_str_to_str(path.as_ref())
}
pub fn os_str_to_str(os_str: &OsStr) -> Cow<str> {
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<OsStr>` to comma separated String
///
/// Panics if the slice is empty.
pub fn pretty_format_list_of_paths(os_strs: &[impl AsRef<Path>]) -> String {
let mut iter = os_strs.iter().map(AsRef::as_ref);
pub fn pretty_format_list_of_paths(paths: &[impl AsRef<Path>]) -> 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<str> {
if path == Path::new(".") {
Cow::Borrowed("current directory")
} else {
to_utf(path)
path_to_str(path)
}
}

View File

@ -37,6 +37,7 @@ fn info_with_accessibility(contents: String, accessible: bool) {
});
}
#[track_caller]
pub fn warning(contents: String) {
logger_thread::send_log_message(PrintMessage {
contents,
@ -147,6 +148,16 @@ mod logger_thread {
}
}
#[cfg(test)]
// shutdown_and_wait must be called manually, but to keep 'em clean, in
// case of tests just do it on drop
impl Drop for LoggerThreadHandle {
fn drop(&mut self) {
send_shutdown_message();
self.shutdown_barrier.wait();
}
}
pub fn spawn_logger_thread() -> LoggerThreadHandle {
let log_receiver = setup_channel();

View File

@ -11,18 +11,19 @@ pub mod io;
pub mod logger;
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_path_stdin, 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_path_stdin, 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};

View File

@ -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, io::lock_and_flush_output_stdio},
utils::{self, colors, formatting::path_to_str, io::lock_and_flush_output_stdio, 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)

View File

@ -6,7 +6,7 @@ use std::{iter::once, path::PathBuf};
use fs_err as fs;
use parse_display::Display;
use proptest::sample::size_range;
use rand::{rngs::SmallRng, Rng, RngCore as _, SeedableRng};
use rand::{rngs::SmallRng, Rng, SeedableRng};
use tempfile::tempdir;
use test_strategy::{proptest, Arbitrary};

View File

@ -0,0 +1,14 @@
---
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, rar, 7z
hint: Supported aliases are: tgz, tbz, tlz4, txz, tzlma, tsz, tzst
hint:
hint: Examples:
hint: --format tar
hint: --format gz
hint: --format tar.gz

View File

@ -0,0 +1,14 @@
---
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, rar, 7z
hint: Supported aliases are: tgz, tbz, tlz4, txz, tzlma, tsz, tzst
hint:
hint: Examples:
hint: --format tar
hint: --format gz
hint: --format tar.gz

View File

@ -0,0 +1,14 @@
---
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, rar, 7z
hint: Supported aliases are: tgz, tbz, tlz4, txz, tzlma, tsz, tzst
hint:
hint: Examples:
hint: --format tar
hint: --format gz
hint: --format tar.gz

View File

@ -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'.

View File

@ -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'.

View File

@ -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
create_files_in(dir, &["input"]);
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
create_files_in(dir, &["input"]);
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();

View File

@ -52,6 +52,7 @@ pub fn create_files_in(dir: &Path, files: &[&str]) {
/// Write random content to a file
pub fn write_random_content(file: &mut impl Write, rng: &mut impl RngCore) {
let mut data = vec![0; rng.gen_range(0..4096)];
rng.fill_bytes(&mut data);
file.write_all(&data).unwrap();
}