Merge fbefb54b0269b9cf225ba2d39f51892c9f9c21c3 into 11344a6ffd58d8f16dc859a313aaa14421398d81

This commit is contained in:
tommady 2025-05-22 06:41:37 +00:00 committed by GitHub
commit 2f73fb0b72
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 159 additions and 70 deletions

View File

@ -1,4 +1,5 @@
use std::{
collections::HashMap,
io::{self, BufReader, Read},
ops::ControlFlow,
path::{Path, PathBuf},
@ -22,7 +23,7 @@ use crate::{
logger::{info, info_accessible},
nice_directory_display, user_wants_to_continue,
},
QuestionAction, QuestionPolicy, BUFFER_CAPACITY,
FileConflitOperation, QuestionAction, QuestionPolicy, BUFFER_CAPACITY,
};
trait ReadSeek: Read + io::Seek {}
@ -341,19 +342,27 @@ fn unpack(
let is_valid_output_dir = !output_dir.exists() || (output_dir.is_dir() && output_dir.read_dir()?.next().is_none());
let output_dir_cleaned = if is_valid_output_dir {
output_dir.to_owned()
output_dir
} else {
match utils::resolve_path_conflict(output_dir, question_policy, QuestionAction::Decompression)? {
Some(path) => path,
None => return Ok(ControlFlow::Break(())),
match utils::check_conflics_and_ask_user(output_dir, question_policy, QuestionAction::Decompression)? {
FileConflitOperation::Cancel => return Ok(ControlFlow::Break(())),
FileConflitOperation::Overwrite => {
// TODO: issue 820 could try to enhance here to fix the issue
// https://github.com/ouch-org/ouch/issues/820
utils::remove_file_or_dir(output_dir)?;
output_dir
}
FileConflitOperation::Rename => &utils::rename_for_available_filename(output_dir),
FileConflitOperation::Merge => output_dir,
FileConflitOperation::GoodToGo => output_dir,
}
};
if !output_dir_cleaned.exists() {
fs::create_dir(&output_dir_cleaned)?;
fs::create_dir(output_dir_cleaned)?;
}
let files = unpack_fn(&output_dir_cleaned)?;
let files = unpack_fn(output_dir_cleaned)?;
Ok(ControlFlow::Continue(files))
}
@ -383,7 +392,7 @@ fn smart_unpack(
let root_contains_only_one_element = fs::read_dir(temp_dir_path)?.take(2).count() == 1;
let (previous_path, mut new_path) = if root_contains_only_one_element {
let (previous_path, new_path) = if root_contains_only_one_element {
// Only one file in the root directory, so we can just move it to the output directory
let file = fs::read_dir(temp_dir_path)?.next().expect("item exists")?;
let file_path = file.path();
@ -397,15 +406,49 @@ fn smart_unpack(
(temp_dir_path.to_owned(), output_file_path.to_owned())
};
// 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, QuestionAction::Decompression)? {
Some(path) => path,
None => return Ok(ControlFlow::Break(())),
};
match utils::check_conflics_and_ask_user(&new_path, question_policy, QuestionAction::Decompression)? {
FileConflitOperation::Cancel => return Ok(ControlFlow::Break(())),
FileConflitOperation::GoodToGo => {
fs::rename(&previous_path, &new_path)?;
}
FileConflitOperation::Overwrite => {
// TODO: issue 820 could try to enhance here to fix the issue
// https://github.com/ouch-org/ouch/issues/820
utils::remove_file_or_dir(&new_path)?;
fs::rename(&previous_path, &new_path)?;
}
FileConflitOperation::Rename => {
fs::rename(&previous_path, utils::rename_for_available_filename(&new_path))?;
}
FileConflitOperation::Merge => {
// TODO: just a simple way to verify, will enhance later
let mut seen = HashMap::new();
for entry in fs::read_dir(&new_path)? {
let entry = entry?;
let path = entry.path();
let name = path
.file_name()
.expect("The paths were read by the program, so it should be safe.");
seen.insert(name.to_os_string(), path);
}
for entry in fs::read_dir(&previous_path)? {
let entry = entry?;
let path = entry.path();
let name = path
.file_name()
.expect("The paths were read by the program, so it should be safe.");
if seen.contains_key(&name.to_os_string()) {
continue;
}
fs::copy(&path, new_path.join(name))?;
}
}
}
// Rename the temporary directory to the archive name, which is output_file_path
fs::rename(&previous_path, &new_path)?;
info_accessible(format!(
"Successfully moved \"{}\" to \"{}\"",
nice_directory_display(&previous_path),

View File

@ -17,7 +17,7 @@ use self::{
error::{Error, Result},
utils::{
logger::{shutdown_logger_and_wait, spawn_logger_thread},
QuestionAction, QuestionPolicy,
FileConflitOperation, QuestionAction, QuestionPolicy,
},
};

View File

@ -8,47 +8,15 @@ use std::{
use fs_err as fs;
use super::{question::FileConflitOperation, user_wants_to_overwrite};
use crate::{
extension::Extension,
utils::{logger::info_accessible, EscapedPathDisplay, QuestionAction},
QuestionPolicy,
utils::{logger::info_accessible, EscapedPathDisplay},
};
pub fn is_path_stdin(path: &Path) -> bool {
path.as_os_str() == "-"
}
/// Check if &Path exists, if it does then ask the user if they want to overwrite or rename it.
/// If the user want to overwrite then the file or directory will be removed and returned the same input path
/// If the user want to rename then nothing will be removed and a new path will be returned with a new name
///
/// * `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,
question_action: QuestionAction,
) -> crate::Result<Option<PathBuf>> {
if path.exists() {
match user_wants_to_overwrite(path, question_policy, question_action)? {
FileConflitOperation::Cancel => Ok(None),
FileConflitOperation::Overwrite => {
remove_file_or_dir(path)?;
Ok(Some(path.to_path_buf()))
}
FileConflitOperation::Rename => {
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()))
}
}
pub fn remove_file_or_dir(path: &Path) -> crate::Result<()> {
if path.is_dir() {
fs::remove_dir_all(path)?;

View File

@ -19,11 +19,11 @@ pub use self::{
},
fs::{
cd_into_same_dir_as, create_dir_if_non_existent, is_path_stdin, remove_file_or_dir,
rename_for_available_filename, resolve_path_conflict, try_infer_extension,
rename_for_available_filename, try_infer_extension,
},
question::{
ask_to_create_file, user_wants_to_continue, user_wants_to_overwrite, FileConflitOperation, QuestionAction,
QuestionPolicy,
ask_to_create_file, check_conflics_and_ask_user, user_wants_to_continue, user_wants_to_overwrite,
FileConflitOperation, QuestionAction, QuestionPolicy,
},
utf8::{get_invalid_utf8_paths, is_invalid_utf8},
};

View File

@ -50,6 +50,41 @@ pub enum FileConflitOperation {
Rename,
/// Merge conflicting folders
Merge,
/// There is no conflict, good to go
GoodToGo,
}
/// Check if &Path exists, if it does then ask the user what to do.
/// If the user want to overwrite then the file or directory will be removed and returned the same input path
/// If the user want to rename then nothing will be removed and a new path will be returned with a new name
/// If the user want to merge then the conflics file or directory will be skiped
///
/// * `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 check_conflics_and_ask_user(
path: &Path,
question_policy: QuestionPolicy,
question_action: QuestionAction,
) -> crate::Result<FileConflitOperation> {
if path.exists() {
user_wants_to_overwrite(path, question_policy, question_action)
// match user_wants_to_overwrite(path, question_policy, question_action)? {
// FileConflitOperation::Cancel => Ok(None),
// FileConflitOperation::Overwrite => {
// remove_file_or_dir(path)?;
// Ok(Some(path.to_path_buf()))
// }
// FileConflitOperation::Rename => {
// let renamed_path = rename_for_available_filename(path);
// Ok(Some(renamed_path))
// }
// FileConflitOperation::Merge => Ok(Some(path.to_path_buf())),
// }
} else {
Ok(FileConflitOperation::GoodToGo)
// Ok(Some(path.to_path_buf()))
}
}
/// Check if QuestionPolicy flags were set, otherwise, ask user if they want to overwrite.
@ -112,7 +147,7 @@ pub fn ask_to_create_file(
};
match action {
FileConflitOperation::Merge => Ok(Some(fs::File::create(path)?)),
FileConflitOperation::Merge | FileConflitOperation::GoodToGo => Ok(Some(fs::File::create(path)?)),
FileConflitOperation::Overwrite => {
utils::remove_file_or_dir(path)?;
Ok(Some(fs::File::create(path)?))

View File

@ -814,35 +814,63 @@ fn unpack_multiple_sources_into_the_same_destination_with_merge(
) {
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");
let source_path = root_path.join(format!("example_{}", merge_extensions(&ext, &extra_extensions)));
fs::create_dir_all(&source_path)?;
fs::File::create(source_path.join("file1.txt"))?;
fs::File::create(source_path.join("file2.txt"))?;
fs::File::create(source_path.join("file3.txt"))?;
let sub_folder_path = source_path.join("sub_a");
fs::create_dir_all(&sub_folder_path)?;
fs::File::create(sub_folder_path.join("file1.txt"))?;
fs::File::create(sub_folder_path.join("file2.txt"))?;
fs::File::create(sub_folder_path.join("file3.txt"))?;
let sub_folder_path = sub_folder_path.join("sub_b");
fs::create_dir_all(&sub_folder_path)?;
fs::File::create(sub_folder_path.join("file1.txt"))?;
fs::File::create(sub_folder_path.join("file2.txt"))?;
fs::File::create(sub_folder_path.join("file3.txt"))?;
let sub_folder_path = sub_folder_path.join("sub_c");
fs::create_dir_all(&sub_folder_path)?;
fs::File::create(sub_folder_path.join("file1.txt"))?;
fs::File::create(sub_folder_path.join("file2.txt"))?;
fs::File::create(sub_folder_path.join("file3.txt"))?;
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(&source_path)
.arg(&archive)
.assert()
.success();
fs::remove_dir_all(&source_path)?;
let source_path = root_path.join(format!("example_{}", merge_extensions(&ext, &extra_extensions)));
fs::create_dir_all(&source_path)?;
fs::File::create(source_path.join("file3.txt"))?;
fs::File::create(source_path.join("file4.txt"))?;
fs::File::create(source_path.join("file5.txt"))?;
let sub_folder_path = source_path.join("sub_a");
fs::create_dir_all(&sub_folder_path)?;
fs::File::create(sub_folder_path.join("file3.txt"))?;
fs::File::create(sub_folder_path.join("file4.txt"))?;
fs::File::create(sub_folder_path.join("file5.txt"))?;
let sub_folder_path = sub_folder_path.join("sub_b");
fs::create_dir_all(&sub_folder_path)?;
fs::File::create(sub_folder_path.join("file3.txt"))?;
fs::File::create(sub_folder_path.join("file4.txt"))?;
fs::File::create(sub_folder_path.join("file5.txt"))?;
let sub_folder_path = sub_folder_path.join("sub_c");
fs::create_dir_all(&sub_folder_path)?;
fs::File::create(sub_folder_path.join("file3.txt"))?;
fs::File::create(sub_folder_path.join("file4.txt"))?;
fs::File::create(sub_folder_path.join("file5.txt"))?;
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(&source_path)
.arg(&archive1)
.assert()
.success();
@ -867,7 +895,22 @@ fn unpack_multiple_sources_into_the_same_destination_with_merge(
.assert()
.success();
assert_eq!(5, out_path.as_path().read_dir()?.count());
fn count_files_recursively(dir: &Path) -> usize {
let mut count = 0;
if let Ok(entries) = fs::read_dir(dir) {
for entry in entries.flatten() {
let path = entry.path();
if path.is_file() {
count += 1;
} else if path.is_dir() {
count += count_files_recursively(&path);
}
}
}
count
}
assert_eq!(20, count_files_recursively(&out_path));
}
#[test]