diff --git a/CHANGELOG.md b/CHANGELOG.md index 97d8057..734e684 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,7 +21,10 @@ Categories Used: ## [Unreleased](https://github.com/ouch-org/ouch/compare/0.6.1...HEAD) ### New Features + +- Merge folders in decompression [\#798](https://github.com/ouch-org/ouch/pull/798) ([tommady](https://github.com/tommady)) - Add `--no-smart-unpack` flag to decompression command to disable smart unpack [\#809](https://github.com/ouch-org/ouch/pull/809) ([talis-fb](https://github.com/talis-fb)) + ### Improvements ### Bug Fixes ### Tweaks diff --git a/src/archive/rar.rs b/src/archive/rar.rs index 37894a0..0846dae 100644 --- a/src/archive/rar.rs +++ b/src/archive/rar.rs @@ -18,8 +18,6 @@ pub fn unpack_archive( password: Option<&[u8]>, quiet: bool, ) -> crate::Result { - assert!(output_folder.read_dir().expect("dir exists").next().is_none()); - let archive = match password { Some(password) => Archive::with_password(archive_path, password), None => Archive::new(archive_path), diff --git a/src/archive/tar.rs b/src/archive/tar.rs index 513d36c..4f457fe 100644 --- a/src/archive/tar.rs +++ b/src/archive/tar.rs @@ -24,7 +24,6 @@ use crate::{ /// Unpacks the archive given by `archive` into the folder given by `into`. /// Assumes that output_folder is empty pub fn unpack_archive(reader: Box, output_folder: &Path, quiet: bool) -> crate::Result { - assert!(output_folder.read_dir().expect("dir exists").next().is_none()); let mut archive = tar::Archive::new(reader); let mut files_unpacked = 0; diff --git a/src/archive/zip.rs b/src/archive/zip.rs index 9beac49..28546a1 100644 --- a/src/archive/zip.rs +++ b/src/archive/zip.rs @@ -37,8 +37,6 @@ pub fn unpack_archive( where R: Read + Seek, { - assert!(output_folder.read_dir().expect("dir exists").next().is_none()); - let mut unpacked_files = 0; for idx in 0..archive.len() { diff --git a/src/commands/decompress.rs b/src/commands/decompress.rs index 5c264ef..36c8658 100644 --- a/src/commands/decompress.rs +++ b/src/commands/decompress.rs @@ -139,7 +139,11 @@ pub fn decompress_file(options: DecompressOptions) -> crate::Result<()> { Gzip | Bzip | Bzip3 | Lz4 | Lzma | Snappy | Zstd | Brotli => { reader = chain_reader_decoder(&first_extension, reader)?; - let mut writer = match utils::ask_to_create_file(&options.output_file_path, options.question_policy)? { + let mut writer = match utils::ask_to_create_file( + &options.output_file_path, + options.question_policy, + QuestionAction::Decompression, + )? { Some(file) => file, None => return Ok(()), }; @@ -331,7 +335,7 @@ fn unpack( let output_dir_cleaned = if is_valid_output_dir { output_dir.to_owned() } else { - match utils::resolve_path_conflict(output_dir, question_policy)? { + match utils::resolve_path_conflict(output_dir, question_policy, QuestionAction::Decompression)? { Some(path) => path, None => return Ok(ControlFlow::Break(())), } @@ -387,7 +391,7 @@ fn smart_unpack( // Before moving, need to check if a file with the same name already exists // If it does, need to ask the user what to do - new_path = match utils::resolve_path_conflict(&new_path, question_policy)? { + new_path = match utils::resolve_path_conflict(&new_path, question_policy, QuestionAction::Decompression)? { Some(path) => path, None => return Ok(ControlFlow::Break(())), }; diff --git a/src/commands/mod.rs b/src/commands/mod.rs index dadfd95..c3363d1 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -20,6 +20,7 @@ use crate::{ list::ListOptions, utils::{ self, colors::*, is_path_stdin, logger::info_accessible, path_to_str, EscapedPathDisplay, FileVisibilityPolicy, + QuestionAction, }, CliArgs, QuestionPolicy, }; @@ -91,10 +92,11 @@ pub fn run( )?; check::check_archive_formats_position(&formats, &output_path)?; - let output_file = match utils::ask_to_create_file(&output_path, question_policy)? { - Some(writer) => writer, - None => return Ok(()), - }; + let output_file = + match utils::ask_to_create_file(&output_path, question_policy, QuestionAction::Compression)? { + Some(writer) => writer, + None => return Ok(()), + }; let level = if fast { Some(1) // Lowest level of compression diff --git a/src/utils/fs.rs b/src/utils/fs.rs index c6ffb8a..eb40625 100644 --- a/src/utils/fs.rs +++ b/src/utils/fs.rs @@ -11,7 +11,7 @@ use fs_err as fs; use super::{question::FileConflitOperation, user_wants_to_overwrite}; use crate::{ extension::Extension, - utils::{logger::info_accessible, EscapedPathDisplay}, + utils::{logger::info_accessible, EscapedPathDisplay, QuestionAction}, QuestionPolicy, }; @@ -26,9 +26,13 @@ pub fn is_path_stdin(path: &Path) -> bool { /// * `Ok(None)` means the user wants to cancel the operation /// * `Ok(Some(path))` returns a valid PathBuf without any another file or directory with the same name /// * `Err(_)` is an error -pub fn resolve_path_conflict(path: &Path, question_policy: QuestionPolicy) -> crate::Result> { +pub fn resolve_path_conflict( + path: &Path, + question_policy: QuestionPolicy, + question_action: QuestionAction, +) -> crate::Result> { if path.exists() { - match user_wants_to_overwrite(path, question_policy)? { + match user_wants_to_overwrite(path, question_policy, question_action)? { FileConflitOperation::Cancel => Ok(None), FileConflitOperation::Overwrite => { remove_file_or_dir(path)?; @@ -38,6 +42,7 @@ pub fn resolve_path_conflict(path: &Path, question_policy: QuestionPolicy) -> cr let renamed_path = rename_for_available_filename(path); Ok(Some(renamed_path)) } + FileConflitOperation::Merge => Ok(Some(path.to_path_buf())), } } else { Ok(Some(path.to_path_buf())) diff --git a/src/utils/question.rs b/src/utils/question.rs index 27eb7bb..4b4c97e 100644 --- a/src/utils/question.rs +++ b/src/utils/question.rs @@ -48,49 +48,71 @@ pub enum FileConflitOperation { /// Rename the file /// It'll be put "_1" at the end of the filename or "_2","_3","_4".. if already exists Rename, + /// Merge conflicting folders + Merge, } /// Check if QuestionPolicy flags were set, otherwise, ask user if they want to overwrite. -pub fn user_wants_to_overwrite(path: &Path, question_policy: QuestionPolicy) -> crate::Result { +pub fn user_wants_to_overwrite( + path: &Path, + question_policy: QuestionPolicy, + question_action: QuestionAction, +) -> crate::Result { use FileConflitOperation as Op; match question_policy { QuestionPolicy::AlwaysYes => Ok(Op::Overwrite), QuestionPolicy::AlwaysNo => Ok(Op::Cancel), - QuestionPolicy::Ask => ask_file_conflict_operation(path), + QuestionPolicy::Ask => ask_file_conflict_operation(path, question_action), } } /// Ask the user if they want to overwrite or rename the &Path -pub fn ask_file_conflict_operation(path: &Path) -> Result { +pub fn ask_file_conflict_operation(path: &Path, question_action: QuestionAction) -> Result { use FileConflitOperation as Op; let path = path_to_str(strip_cur_dir(path)); - - ChoicePrompt::new( - format!("Do you want to overwrite {path}?"), - [ - ("yes", Op::Overwrite, *colors::GREEN), - ("no", Op::Cancel, *colors::RED), - ("rename", Op::Rename, *colors::BLUE), - ], - ) - .ask() + match question_action { + QuestionAction::Compression => ChoicePrompt::new( + format!("Do you want to overwrite {path}?"), + [ + ("yes", Op::Overwrite, *colors::GREEN), + ("no", Op::Cancel, *colors::RED), + ("rename", Op::Rename, *colors::BLUE), + ], + ) + .ask(), + QuestionAction::Decompression => ChoicePrompt::new( + format!("Do you want to overwrite {path}?"), + [ + ("yes", Op::Overwrite, *colors::GREEN), + ("no", Op::Cancel, *colors::RED), + ("rename", Op::Rename, *colors::BLUE), + ("merge", Op::Merge, *colors::ORANGE), + ], + ) + .ask(), + } } /// Create the file if it doesn't exist and if it does then ask to overwrite it. /// If the user doesn't want to overwrite then we return [`Ok(None)`] -pub fn ask_to_create_file(path: &Path, question_policy: QuestionPolicy) -> Result> { +pub fn ask_to_create_file( + path: &Path, + question_policy: QuestionPolicy, + question_action: QuestionAction, +) -> Result> { match fs::OpenOptions::new().write(true).create_new(true).open(path) { Ok(w) => Ok(Some(w)), Err(e) if e.kind() == std::io::ErrorKind::AlreadyExists => { let action = match question_policy { QuestionPolicy::AlwaysYes => FileConflitOperation::Overwrite, QuestionPolicy::AlwaysNo => FileConflitOperation::Cancel, - QuestionPolicy::Ask => ask_file_conflict_operation(path)?, + QuestionPolicy::Ask => ask_file_conflict_operation(path, question_action)?, }; match action { + FileConflitOperation::Merge => Ok(Some(fs::File::create(path)?)), FileConflitOperation::Overwrite => { utils::remove_file_or_dir(path)?; Ok(Some(fs::File::create(path)?)) diff --git a/tests/integration.rs b/tests/integration.rs index ea82326..f85e254 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -59,7 +59,7 @@ enum Extension { } /// Converts a list of extension structs to string -fn merge_extensions(ext: impl ToString, exts: Vec) -> String { +fn merge_extensions(ext: impl ToString, exts: &Vec) -> String { once(ext.to_string()) .chain(exts.into_iter().map(|x| x.to_string())) .collect::>() @@ -114,7 +114,7 @@ fn single_empty_file(ext: Extension, #[any(size_range(0..8).lift())] exts: Vec>(); - let archive = &root_path.join(format!("archive.{}", merge_extensions(&ext, extra_extensions))); + let archive = &root_path.join(format!("archive.{}", merge_extensions(&ext, &extra_extensions))); crate::utils::cargo_bin() .arg("compress") @@ -438,7 +438,7 @@ fn smart_unpack_with_multiple_files( .map(|entry| entry.unwrap().path()) .collect::>(); - let archive = &root_path.join(format!("archive.{}", merge_extensions(&ext, extra_extensions))); + let archive = &root_path.join(format!("archive.{}", merge_extensions(&ext, &extra_extensions))); let output_path = root_path.join("archive"); assert!(!output_path.exists()); @@ -487,7 +487,7 @@ fn no_smart_unpack_with_single_file( .map(|entry| entry.unwrap().path()) .collect::>(); - let archive = &root_path.join(format!("archive.{}", merge_extensions(&ext, extra_extensions))); + let archive = &root_path.join(format!("archive.{}", merge_extensions(&ext, &extra_extensions))); let output_path = root_path.join("archive"); assert!(!output_path.exists()); @@ -537,7 +537,7 @@ fn no_smart_unpack_with_multiple_files( .map(|entry| entry.unwrap().path()) .collect::>(); - let archive = &root_path.join(format!("archive.{}", merge_extensions(&ext, extra_extensions))); + let archive = &root_path.join(format!("archive.{}", merge_extensions(&ext, &extra_extensions))); let output_path = root_path.join("archive"); assert!(!output_path.exists()); @@ -585,7 +585,7 @@ fn multiple_files_with_disabled_smart_unpack_by_dir( let dest_files_path = root_path.join("dest_files"); fs::create_dir_all(&dest_files_path).unwrap(); - let archive = &root_path.join(format!("archive.{}", merge_extensions(&ext, extra_extensions))); + let archive = &root_path.join(format!("archive.{}", merge_extensions(&ext, &extra_extensions))); crate::utils::cargo_bin() .arg("compress") @@ -702,7 +702,7 @@ fn symlink_pack_and_unpack( files_path.push(symlink_path); - let archive = &root_path.join(format!("archive.{}", merge_extensions(&ext, extra_extensions))); + let archive = &root_path.join(format!("archive.{}", merge_extensions(&ext, &extra_extensions))); crate::utils::cargo_bin() .arg("compress") @@ -798,3 +798,67 @@ fn no_git_folder_after_decompression_with_gitignore_flag_active() { ".git folder should not exist after decompression" ); } + +#[cfg(feature = "allow_piped_choice")] +#[proptest(cases = 25)] +fn unpack_multiple_sources_into_the_same_destination_with_merge( + ext: DirectoryExtension, + #[any(size_range(0..1).lift())] extra_extensions: Vec, +) { + let temp_dir = tempdir()?; + let root_path = temp_dir.path(); + let source_path = root_path + .join(format!("example_{}", merge_extensions(&ext, &extra_extensions))) + .join("sub_a") + .join("sub_b") + .join("sub_c"); + + fs::create_dir_all(&source_path)?; + let archive = root_path.join(format!("archive.{}", merge_extensions(&ext, &extra_extensions))); + crate::utils::cargo_bin() + .arg("compress") + .args([ + fs::File::create(source_path.join("file1.txt"))?.path(), + fs::File::create(source_path.join("file2.txt"))?.path(), + fs::File::create(source_path.join("file3.txt"))?.path(), + ]) + .arg(&archive) + .assert() + .success(); + + fs::remove_dir_all(&source_path)?; + fs::create_dir_all(&source_path)?; + let archive1 = root_path.join(format!("archive1.{}", merge_extensions(&ext, &extra_extensions))); + crate::utils::cargo_bin() + .arg("compress") + .args([ + fs::File::create(source_path.join("file3.txt"))?.path(), + fs::File::create(source_path.join("file4.txt"))?.path(), + fs::File::create(source_path.join("file5.txt"))?.path(), + ]) + .arg(&archive1) + .assert() + .success(); + + let out_path = root_path.join(format!("out_{}", merge_extensions(&ext, &extra_extensions))); + fs::create_dir_all(&out_path)?; + + crate::utils::cargo_bin() + .arg("decompress") + .arg(archive) + .arg("-d") + .arg(&out_path) + .assert() + .success(); + + crate::utils::cargo_bin() + .arg("decompress") + .arg(archive1) + .arg("-d") + .arg(&out_path) + .write_stdin("m") + .assert() + .success(); + + assert_eq!(5, out_path.as_path().read_dir()?.count()); +}