scaffold close-issue-825

Signed-off-by: tommady <tommady@users.noreply.github.com>

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)? {
+        // TODO: will enhance later
-            Some(path) => path,
+        match utils::check_conflics_and_ask_user(output_dir, question_policy, QuestionAction::Decompression)? {
-            None => return Ok(ControlFlow::Break(())),
+            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
+    // TODO: will enhance later
-    // If it does, need to ask the user what to do
+    match utils::check_conflics_and_ask_user(&new_path, question_policy, QuestionAction::Decompression)? {
-    new_path = match utils::resolve_path_conflict(&new_path, question_policy, QuestionAction::Decompression)? {
+        FileConflitOperation::Cancel => return Ok(ControlFlow::Break(())),
-        Some(path) => path,
+        FileConflitOperation::GoodToGo => {
-        None => return Ok(ControlFlow::Break(())),
-    };
-
-    // Rename the temporary directory to the archive name, which is output_file_path
             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))?;
+            }
+        }
+    }
+
     info_accessible(format!(
         "Successfully moved \"{}\" to \"{}\"",
         nice_directory_display(&previous_path),

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,
     },
 };
 

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},
+    utils::{logger::info_accessible, EscapedPathDisplay},
-    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<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)?;

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,
+        ask_to_create_file, check_conflics_and_ask_user, user_wants_to_continue, user_wants_to_overwrite,
-        QuestionPolicy,
+        FileConflitOperation, QuestionAction, QuestionPolicy,
     },
     utf8::{get_invalid_utf8_paths, is_invalid_utf8},
 };

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<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)?))