feat: Add rename option in overwrite menu (#779)

* feat: Add generic Choice prompt implementation

* feat: use ChoicePrompt in user_wants_to_continue method

* feat: add "rename" option in ask_to_create_file method

* feat: check accessible mode for choises prompt

* feat: rename file in "ask_to_create_file" rename action

* feat: create "resolve_path" for smart_unpack to deal with rename

* feat: use resolve_path instead clear_path in smart_unpack

* fix: remove unused clear_path function

* chore: cargo fmt

* Add docs

* refactor: rename "resolve_path" method

* chore: fix ChoicePrompt doc

* doc: improve doc of resolve_path_conflict

* fix: out of bound when type answer bigger than some choice

* doc: improve rename_path docs

* chore: cargo fmt

* chore: revert user_wants_to_continue

* fix: update error message when find EOF in choise prompt response

* revert: update message error in ChoicePrompt instead Confirmation

* test: add overwrite and cancel tests

* test: Add rename test with "allow_piped_choice" new feature

* cargo fmt

* test: create test for autoincrement new renamed files

Cargo.toml

@@ -73,6 +73,7 @@ test-strategy = "0.4.0"
 default = ["use_zlib", "use_zstd_thin", "unrar"]
 use_zlib = ["flate2/zlib", "gzp/deflate_zlib", "zip/deflate-zlib"]
 use_zstd_thin = ["zstd/thin"]
+allow_piped_choice = []
 
 # For generating binaries for releases
 [profile.release]

src/commands/decompress.rs

@@ -309,7 +309,7 @@ fn smart_unpack(
 
     let root_contains_only_one_element = fs::read_dir(temp_dir_path)?.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();
@@ -324,9 +324,11 @@ fn smart_unpack(
     };
 
     // Before moving, need to check if a file with the same name already exists
-    if !utils::clear_path(&new_path, question_policy)? {
+    // If it does, need to ask the user what to do
-        return Ok(ControlFlow::Break(()));
+    new_path = match utils::resolve_path_conflict(&new_path, question_policy)? {
-    }
+        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)?;

src/utils/fs.rs

@@ -8,7 +8,7 @@ use std::{
 
 use fs_err as fs;
 
-use super::user_wants_to_overwrite;
+use super::{question::FileConflitOperation, user_wants_to_overwrite};
 use crate::{
     extension::Extension,
     utils::{logger::info_accessible, EscapedPathDisplay},
@@ -19,19 +19,29 @@ pub fn is_path_stdin(path: &Path) -> bool {
     path.as_os_str() == "-"
 }
 
-/// Remove `path` asking the user to overwrite if necessary.
+/// 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(true)` means the path is clear,
+/// * `Ok(None)` means the user wants to cancel the operation
-/// * `Ok(false)` means the user doesn't want to overwrite
+/// * `Ok(Some(path))` returns a valid PathBuf without any another file or directory with the same name
 /// * `Err(_)` is an error
-pub fn clear_path(path: &Path, question_policy: QuestionPolicy) -> crate::Result<bool> {
+pub fn resolve_path_conflict(path: &Path, question_policy: QuestionPolicy) -> crate::Result<Option<PathBuf>> {
-    if path.exists() && !user_wants_to_overwrite(path, question_policy)? {
+    if path.exists() {
-        return Ok(false);
+        match user_wants_to_overwrite(path, question_policy)? {
+            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))
+            }
+        }
+    } else {
+        Ok(Some(path.to_path_buf()))
     }
-
-    remove_file_or_dir(path)?;
-
-    Ok(true)
 }
 
 pub fn remove_file_or_dir(path: &Path) -> crate::Result<()> {
@@ -43,6 +53,41 @@ pub fn remove_file_or_dir(path: &Path) -> crate::Result<()> {
     Ok(())
 }
 
+/// Create a new path renaming the "filename" from &Path for a available name in the same directory
+pub fn rename_for_available_filename(path: &Path) -> PathBuf {
+    let mut renamed_path = rename_or_increment_filename(path);
+    while renamed_path.exists() {
+        renamed_path = rename_or_increment_filename(&renamed_path);
+    }
+    renamed_path
+}
+
+/// Create a new path renaming the "filename" from &Path to `filename_1`
+/// if its name already ends with `_` and some number, then it increments the number
+/// Example:
+/// - `file.txt` -> `file_1.txt`
+/// - `file_1.txt` -> `file_2.txt`
+pub fn rename_or_increment_filename(path: &Path) -> PathBuf {
+    let parent = path.parent().unwrap_or_else(|| Path::new(""));
+    let filename = path.file_stem().and_then(|s| s.to_str()).unwrap_or("");
+    let extension = path.extension().and_then(|s| s.to_str()).unwrap_or("");
+
+    let new_filename = match filename.rsplit_once('_') {
+        Some((base, number_str)) if number_str.chars().all(char::is_numeric) => {
+            let number = number_str.parse::<u32>().unwrap_or(0);
+            format!("{}_{}", base, number + 1)
+        }
+        _ => format!("{}_1", filename),
+    };
+
+    let mut new_path = parent.join(new_filename);
+    if !extension.is_empty() {
+        new_path.set_extension(extension);
+    }
+
+    new_path
+}
+
 /// Creates a directory at the path, if there is nothing there.
 pub fn create_dir_if_non_existent(path: &Path) -> crate::Result<()> {
     if !path.exists() {

src/utils/mod.rs

@@ -18,10 +18,13 @@ pub use self::{
         EscapedPathDisplay,
     },
     fs::{
-        cd_into_same_dir_as, clear_path, create_dir_if_non_existent, is_path_stdin, remove_file_or_dir,
+        cd_into_same_dir_as, create_dir_if_non_existent, is_path_stdin, remove_file_or_dir,
-        try_infer_extension,
+        rename_for_available_filename, resolve_path_conflict, try_infer_extension,
+    },
+    question::{
+        ask_to_create_file, user_wants_to_continue, user_wants_to_overwrite, FileConflitOperation, QuestionAction,
+        QuestionPolicy,
     },
-    question::{ask_to_create_file, user_wants_to_continue, user_wants_to_overwrite, QuestionAction, QuestionPolicy},
     utf8::{get_invalid_utf8_paths, is_invalid_utf8},
 };
 

src/utils/question.rs

@@ -37,31 +37,69 @@ pub enum QuestionAction {
     Decompression,
 }
 
+#[derive(Default)]
+/// Determines which action to do when there is a file conflict
+pub enum FileConflitOperation {
+    #[default]
+    /// Cancel the operation
+    Cancel,
+    /// Overwrite the existing file with the new one
+    Overwrite,
+    /// Rename the file
+    /// It'll be put "_1" at the end of the filename or "_2","_3","_4".. if already exists
+    Rename,
+}
+
 /// 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<bool> {
+pub fn user_wants_to_overwrite(path: &Path, question_policy: QuestionPolicy) -> crate::Result<FileConflitOperation> {
+    use FileConflitOperation as Op;
+
     match question_policy {
-        QuestionPolicy::AlwaysYes => Ok(true),
+        QuestionPolicy::AlwaysYes => Ok(Op::Overwrite),
-        QuestionPolicy::AlwaysNo => Ok(false),
+        QuestionPolicy::AlwaysNo => Ok(Op::Cancel),
-        QuestionPolicy::Ask => {
+        QuestionPolicy::Ask => ask_file_conflict_operation(path),
-            let path = path_to_str(strip_cur_dir(path));
-            let path = Some(&*path);
-            let placeholder = Some("FILE");
-            Confirmation::new("Do you want to overwrite 'FILE'?", placeholder).ask(path)
-        }
     }
 }
 
+/// Ask the user if they want to overwrite or rename the &Path
+pub fn ask_file_conflict_operation(path: &Path) -> Result<FileConflitOperation> {
+    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()
+}
+
 /// 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<Option<fs::File>> {
     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 => {
-            if user_wants_to_overwrite(path, question_policy)? {
+            let action = match question_policy {
-                utils::remove_file_or_dir(path)?;
+                QuestionPolicy::AlwaysYes => FileConflitOperation::Overwrite,
-                Ok(Some(fs::File::create(path)?))
+                QuestionPolicy::AlwaysNo => FileConflitOperation::Cancel,
-            } else {
+                QuestionPolicy::Ask => ask_file_conflict_operation(path)?,
-                Ok(None)
+            };
+
+            match action {
+                FileConflitOperation::Overwrite => {
+                    utils::remove_file_or_dir(path)?;
+                    Ok(Some(fs::File::create(path)?))
+                }
+                FileConflitOperation::Cancel => Ok(None),
+                FileConflitOperation::Rename => {
+                    let renamed_file_path = utils::rename_for_available_filename(path);
+                    Ok(Some(fs::File::create(renamed_file_path)?))
+                }
             }
         }
         Err(e) => Err(Error::from(e)),
@@ -90,6 +128,108 @@ pub fn user_wants_to_continue(
     }
 }
 
+/// Choise dialog for end user with [option1/option2/...] question.
+/// Each option is a [Choice] entity, holding a value "T" returned when that option is selected
+pub struct ChoicePrompt<'a, T: Default> {
+    /// The message to be displayed before the options
+    /// e.g.: "Do you want to overwrite 'FILE'?"
+    pub prompt: String,
+
+    pub choises: Vec<Choice<'a, T>>,
+}
+
+/// A single choice showed as a option to user in a [ChoicePrompt]
+/// It holds a label and a color to display to user and a real value to be returned
+pub struct Choice<'a, T: Default> {
+    label: &'a str,
+    value: T,
+    color: &'a str,
+}
+
+impl<'a, T: Default> ChoicePrompt<'a, T> {
+    /// Creates a new Confirmation.
+    pub fn new(prompt: impl Into<String>, choises: impl IntoIterator<Item = (&'a str, T, &'a str)>) -> Self {
+        Self {
+            prompt: prompt.into(),
+            choises: choises
+                .into_iter()
+                .map(|(label, value, color)| Choice { label, value, color })
+                .collect(),
+        }
+    }
+
+    /// Creates user message and receives a input to be compared with choises "label"
+    /// and returning the real value of the choise selected
+    pub fn ask(mut self) -> crate::Result<T> {
+        let message = self.prompt;
+
+        #[cfg(not(feature = "allow_piped_choice"))]
+        if !stdin().is_terminal() {
+            eprintln!("{}", message);
+            eprintln!("Pass --yes to proceed");
+            return Ok(T::default());
+        }
+
+        let _locks = lock_and_flush_output_stdio()?;
+        let mut stdin_lock = stdin().lock();
+
+        // Ask the same question to end while no valid answers are given
+        loop {
+            let choice_prompt = if is_running_in_accessible_mode() {
+                self.choises
+                    .iter()
+                    .map(|choise| format!("{}{}{}", choise.color, choise.label, *colors::RESET))
+                    .collect::<Vec<_>>()
+                    .join("/")
+            } else {
+                let choises = self
+                    .choises
+                    .iter()
+                    .map(|choise| {
+                        format!(
+                            "{}{}{}",
+                            choise.color,
+                            choise
+                                .label
+                                .chars()
+                                .nth(0)
+                                .expect("dev error, should be reported, we checked this won't happen"),
+                            *colors::RESET
+                        )
+                    })
+                    .collect::<Vec<_>>()
+                    .join("/");
+
+                format!("[{}]", choises)
+            };
+
+            eprintln!("{} {}", message, choice_prompt);
+
+            let mut answer = String::new();
+            let bytes_read = stdin_lock.read_line(&mut answer)?;
+
+            if bytes_read == 0 {
+                let error = FinalError::with_title("Unexpected EOF when asking question.")
+                    .detail("When asking the user:")
+                    .detail(format!("  \"{message}\""))
+                    .detail("Expected one of the options as answer, but found EOF instead.")
+                    .hint("If using Ouch in scripting, consider using `--yes` and `--no`.");
+
+                return Err(error.into());
+            }
+
+            answer.make_ascii_lowercase();
+            let answer = answer.trim();
+
+            let chosen_index = self.choises.iter().position(|choise| choise.label.starts_with(answer));
+
+            if let Some(i) = chosen_index {
+                return Ok(self.choises.remove(i).value);
+            }
+        }
+    }
+}
+
 /// Confirmation dialog for end user with [Y/n] question.
 ///
 /// If the placeholder is found in the prompt text, it will be replaced to form the final message.
@@ -120,6 +260,7 @@ impl<'a> Confirmation<'a> {
             (Some(placeholder), Some(subs)) => Cow::Owned(self.prompt.replace(placeholder, subs)),
         };
 
+        #[cfg(not(feature = "allow_piped_choice"))]
         if !stdin().is_terminal() {
             eprintln!("{}", message);
             eprintln!("Pass --yes to proceed");

tests/integration.rs

@@ -194,6 +194,175 @@ fn multiple_files(
     assert_same_directory(before, after, !matches!(ext, DirectoryExtension::Zip));
 }
 
+#[proptest(cases = 25)]
+fn multiple_files_with_conflict_and_choice_to_overwrite(
+    ext: DirectoryExtension,
+    #[any(size_range(0..1).lift())] extra_extensions: Vec<FileExtension>,
+    #[strategy(0u8..3)] depth: u8,
+) {
+    let dir = tempdir().unwrap();
+    let dir = dir.path();
+
+    let before = &dir.join("before");
+    let before_dir = &before.join("dir");
+    fs::create_dir_all(before_dir).unwrap();
+    create_random_files(before_dir, depth, &mut SmallRng::from_entropy());
+
+    let after = &dir.join("after");
+    let after_dir = &after.join("dir");
+    fs::create_dir_all(after_dir).unwrap();
+    create_random_files(after_dir, depth, &mut SmallRng::from_entropy());
+
+    let archive = &dir.join(format!("archive.{}", merge_extensions(&ext, extra_extensions)));
+    ouch!("-A", "c", before_dir, archive);
+
+    crate::utils::cargo_bin()
+        .arg("decompress")
+        .arg(archive)
+        .arg("-d")
+        .arg(after)
+        .arg("--yes")
+        .assert()
+        .success();
+
+    assert_same_directory(before, after, false);
+}
+
+#[proptest(cases = 25)]
+fn multiple_files_with_conflict_and_choice_to_not_overwrite(
+    ext: DirectoryExtension,
+    #[any(size_range(0..1).lift())] extra_extensions: Vec<FileExtension>,
+    #[strategy(0u8..3)] depth: u8,
+) {
+    let dir = tempdir().unwrap();
+    let dir = dir.path();
+
+    let before = &dir.join("before");
+    let before_dir = &before.join("dir");
+    fs::create_dir_all(before_dir).unwrap();
+    create_random_files(before_dir, depth, &mut SmallRng::from_entropy());
+
+    let after = &dir.join("after");
+    let after_dir = &after.join("dir");
+    fs::create_dir_all(after_dir).unwrap();
+
+    let after_backup = &dir.join("after_backup");
+    let after_backup_dir = &after_backup.join("dir");
+    fs::create_dir_all(after_backup_dir).unwrap();
+
+    // Create a file with the same name as one of the files in the after directory
+    fs::write(after_dir.join("something.txt"), "Some content").unwrap();
+    fs::copy(after_dir.join("something.txt"), after_backup_dir.join("something.txt")).unwrap();
+
+    let archive = &dir.join(format!("archive.{}", merge_extensions(&ext, extra_extensions)));
+    ouch!("-A", "c", before_dir, archive);
+
+    crate::utils::cargo_bin()
+        .arg("decompress")
+        .arg(archive)
+        .arg("-d")
+        .arg(after)
+        .arg("--no")
+        .assert()
+        .success();
+
+    assert_same_directory(after, after_backup, false);
+}
+
+#[cfg(feature = "allow_piped_choice")]
+#[proptest(cases = 25)]
+fn multiple_files_with_conflict_and_choice_to_rename(
+    ext: DirectoryExtension,
+    #[any(size_range(0..1).lift())] extra_extensions: Vec<FileExtension>,
+    #[strategy(0u8..3)] depth: u8,
+) {
+    let dir = tempdir().unwrap();
+    let dir = dir.path();
+
+    let before = &dir.join("before");
+    let before_dir = &before.join("dir");
+    fs::create_dir_all(before_dir).unwrap();
+    create_random_files(before_dir, depth, &mut SmallRng::from_entropy());
+
+    let after = &dir.join("after");
+    let after_dir = &after.join("dir");
+    fs::create_dir_all(after_dir).unwrap();
+    create_random_files(after_dir, depth, &mut SmallRng::from_entropy());
+
+    let archive = &dir.join(format!("archive.{}", merge_extensions(&ext, extra_extensions)));
+    ouch!("-A", "c", before_dir, archive);
+
+    let after_renamed_dir = &after.join("dir_1");
+    assert_eq!(false, after_renamed_dir.exists());
+
+    crate::utils::cargo_bin()
+        .arg("decompress")
+        .arg(archive)
+        .arg("-d")
+        .arg(after)
+        .write_stdin("r")
+        .assert()
+        .success();
+
+    assert_same_directory(before_dir, after_renamed_dir, false);
+}
+
+#[cfg(feature = "allow_piped_choice")]
+#[proptest(cases = 25)]
+fn multiple_files_with_conflict_and_choice_to_rename_with_already_a_renamed(
+    ext: DirectoryExtension,
+    #[any(size_range(0..1).lift())] extra_extensions: Vec<FileExtension>,
+    #[strategy(0u8..3)] depth: u8,
+) {
+    let dir = tempdir().unwrap();
+    let dir = dir.path();
+
+    let before = &dir.join("before");
+    let before_dir = &before.join("dir");
+    fs::create_dir_all(before_dir).unwrap();
+    create_random_files(before_dir, depth, &mut SmallRng::from_entropy());
+
+    let after = &dir.join("after");
+    let after_dir = &after.join("dir");
+    fs::create_dir_all(after_dir).unwrap();
+    create_random_files(after_dir, depth, &mut SmallRng::from_entropy());
+
+    let archive = &dir.join(format!("archive.{}", merge_extensions(&ext, extra_extensions)));
+    ouch!("-A", "c", before_dir, archive);
+
+    let already_renamed_dir = &after.join("dir_1");
+    fs::create_dir_all(already_renamed_dir).unwrap();
+    create_random_files(already_renamed_dir, depth, &mut SmallRng::from_entropy());
+
+    let after_real_renamed_dir = &after.join("dir_2");
+    assert_eq!(false, after_real_renamed_dir.exists());
+
+    crate::utils::cargo_bin()
+        .arg("decompress")
+        .arg(archive)
+        .arg("-d")
+        .arg(after)
+        .write_stdin("r")
+        .assert()
+        .success();
+
+    assert_same_directory(before_dir, after_real_renamed_dir, false);
+
+    let after_another_real_renamed_dir = &after.join("dir_3");
+    assert_eq!(false, after_another_real_renamed_dir.exists());
+
+    crate::utils::cargo_bin()
+        .arg("decompress")
+        .arg(archive)
+        .arg("-d")
+        .arg(after)
+        .write_stdin("r")
+        .assert()
+        .success();
+
+    assert_same_directory(before_dir, after_another_real_renamed_dir, false);
+}
+
 #[cfg(feature = "unrar")]
 #[test]
 fn unpack_rar() -> Result<(), Box<dyn std::error::Error>> {