From dd34293ce8e5a0858f2cdf44e4be5e9ac0b6ffbb Mon Sep 17 00:00:00 2001 From: tommady Date: Sun, 18 May 2025 06:00:51 +0000 Subject: [PATCH] scaffold close-issue-825 Signed-off-by: tommady --- src/commands/decompress.rs | 75 ++++++++++++++++++++++++++++++-------- src/main.rs | 2 +- src/utils/fs.rs | 34 +---------------- src/utils/mod.rs | 6 +-- src/utils/question.rs | 37 ++++++++++++++++++- 5 files changed, 101 insertions(+), 53 deletions(-) diff --git a/src/commands/decompress.rs b/src/commands/decompress.rs index 6c254ad..059bbd3 100644 --- a/src/commands/decompress.rs +++ b/src/commands/decompress.rs @@ -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,16 +342,25 @@ 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(())), + // TODO: will enhance later + 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)?; @@ -383,7 +393,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 +407,50 @@ 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(())), - }; + // TODO: will enhance later + 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), diff --git a/src/main.rs b/src/main.rs index 3f1f7dd..7c784d5 100644 --- a/src/main.rs +++ b/src/main.rs @@ -17,7 +17,7 @@ use self::{ error::{Error, Result}, utils::{ logger::{shutdown_logger_and_wait, spawn_logger_thread}, - QuestionAction, QuestionPolicy, + FileConflitOperation, QuestionAction, QuestionPolicy, }, }; diff --git a/src/utils/fs.rs b/src/utils/fs.rs index f65f152..faa50ba 100644 --- a/src/utils/fs.rs +++ b/src/utils/fs.rs @@ -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> { - 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)?; diff --git a/src/utils/mod.rs b/src/utils/mod.rs index 444cf1f..d157c9f 100644 --- a/src/utils/mod.rs +++ b/src/utils/mod.rs @@ -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}, }; diff --git a/src/utils/question.rs b/src/utils/question.rs index 4b4c97e..5389342 100644 --- a/src/utils/question.rs +++ b/src/utils/question.rs @@ -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 { + 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)?))