From 83003c96ed973535478fbf655c131a50d24d296c Mon Sep 17 00:00:00 2001 From: tommady Date: Sat, 28 Jun 2025 03:20:04 +0000 Subject: [PATCH] Revert "scaffold close-issue-825" This reverts commit dd34293ce8e5a0858f2cdf44e4be5e9ac0b6ffbb. --- src/commands/decompress.rs | 73 ++++++++------------------------------ src/main.rs | 2 +- src/utils/fs.rs | 34 +++++++++++++++++- src/utils/mod.rs | 6 ++-- src/utils/question.rs | 37 +------------------ 5 files changed, 53 insertions(+), 99 deletions(-) diff --git a/src/commands/decompress.rs b/src/commands/decompress.rs index 2dd344e..102d994 100644 --- a/src/commands/decompress.rs +++ b/src/commands/decompress.rs @@ -1,5 +1,4 @@ use std::{ - collections::HashMap, io::{self, BufReader, Read}, ops::ControlFlow, path::{Path, PathBuf}, @@ -23,7 +22,7 @@ use crate::{ logger::{info, info_accessible}, nice_directory_display, user_wants_to_continue, }, - FileConflitOperation, QuestionAction, QuestionPolicy, BUFFER_CAPACITY, + QuestionAction, QuestionPolicy, BUFFER_CAPACITY, }; trait ReadSeek: Read + io::Seek {} @@ -342,24 +341,16 @@ 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 + output_dir.to_owned() } else { - 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, + match utils::resolve_path_conflict(output_dir, question_policy, QuestionAction::Decompression)? { + Some(path) => path, + None => return Ok(ControlFlow::Break(())), } }; if !output_dir_cleaned.exists() { - fs::create_dir(output_dir_cleaned)?; + fs::create_dir(&output_dir_cleaned)?; } let files = unpack_fn(output_dir_cleaned)?; @@ -392,7 +383,7 @@ fn smart_unpack( let root_contains_only_one_element = fs::read_dir(temp_dir_path)?.take(2).count() == 1; - let (previous_path, new_path) = if root_contains_only_one_element { + let (previous_path, mut 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(); @@ -406,49 +397,15 @@ fn smart_unpack( (temp_dir_path.to_owned(), output_file_path.to_owned()) }; - 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))?; - } - } - } + // 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(())), + }; + // 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 7c784d5..3f1f7dd 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}, - FileConflitOperation, QuestionAction, QuestionPolicy, + QuestionAction, QuestionPolicy, }, }; diff --git a/src/utils/fs.rs b/src/utils/fs.rs index faa50ba..f65f152 100644 --- a/src/utils/fs.rs +++ b/src/utils/fs.rs @@ -8,15 +8,47 @@ use std::{ 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, }; 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 d157c9f..444cf1f 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, try_infer_extension, + rename_for_available_filename, resolve_path_conflict, try_infer_extension, }, question::{ - ask_to_create_file, check_conflics_and_ask_user, user_wants_to_continue, user_wants_to_overwrite, - FileConflitOperation, QuestionAction, QuestionPolicy, + ask_to_create_file, 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 5389342..4b4c97e 100644 --- a/src/utils/question.rs +++ b/src/utils/question.rs @@ -50,41 +50,6 @@ 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. @@ -147,7 +112,7 @@ pub fn ask_to_create_file( }; match action { - FileConflitOperation::Merge | FileConflitOperation::GoodToGo => Ok(Some(fs::File::create(path)?)), + FileConflitOperation::Merge => Ok(Some(fs::File::create(path)?)), FileConflitOperation::Overwrite => { utils::remove_file_or_dir(path)?; Ok(Some(fs::File::create(path)?))