cp, mv, and rm commands need to support -i flag (#5523)

* restored interactive mode to rm command

* removed unnecessary whitespace in rm file

* removed unnecessary whitespace in rm file

* fixed python-vertualenv build issue

* moved interactive logic to utils file

* restored interactive mode to cp command

* interactive mode for mv wip

* finished mv implementation

* removed unnecessary whitespace

* changed unwrap to expect

crates/nu-command/src/filesystem/cp.rs

@@ -6,10 +6,12 @@ use nu_path::canonicalize_with;
 use nu_protocol::ast::Call;
 use nu_protocol::engine::{Command, EngineState, Stack};
 use nu_protocol::{
-    Category, Example, IntoInterruptiblePipelineData, PipelineData, ShellError, Signature, Spanned,
+    Category, Example, IntoInterruptiblePipelineData, PipelineData, ShellError, Signature, Span,
-    SyntaxShape, Value,
+    Spanned, SyntaxShape, Value,
 };
 
+use super::util::try_interaction;
+
 use crate::filesystem::util::FileStructure;
 
 const GLOB_PARAMS: nu_glob::MatchOptions = nu_glob::MatchOptions {
@@ -51,7 +53,7 @@ impl Command for Cp {
             )
             // TODO: add back in additional features
             // .switch("force", "suppress error when no file", Some('f'))
-            // .switch("interactive", "ask user to confirm action", Some('i'))
+            .switch("interactive", "ask user to confirm action", Some('i'))
             .category(Category::FileSystem)
     }
 
@@ -66,6 +68,7 @@ impl Command for Cp {
         let dst: Spanned<String> = call.req(engine_state, stack, 1)?;
         let recursive = call.has_flag("recursive");
         let verbose = call.has_flag("verbose");
+        let interactive = call.has_flag("interactive");
 
         let path = current_dir(engine_state, stack)?;
         let source = path.join(src.item.as_str());
@@ -140,25 +143,12 @@ impl Command for Cp {
 
                 for (src, dst) in sources {
                     if src.is_file() {
-                        match std::fs::copy(&src, &dst) {
+                        let res = if interactive && dst.exists() {
-                            Ok(_) => {
+                            interactive_copy_file(interactive, src, dst, span)
-                                let msg =
+                        } else {
-                                    format!("copied {:} to {:}", src.display(), dst.display());
+                            copy_file(src, dst, span)
-                                result.push(Value::String { val: msg, span });
+                        };
-                            }
+                        result.push(res);
-                            Err(e) => {
-                                let error = Value::Error {
-                                    error: ShellError::GenericError(
-                                        e.to_string(),
-                                        e.to_string(),
-                                        Some(span),
-                                        None,
-                                        Vec::new(),
-                                    ),
-                                };
-                                result.push(error);
-                            }
-                        }
                     }
                 }
             } else if entry.is_dir() {
@@ -222,25 +212,12 @@ impl Command for Cp {
                     }
 
                     if s.is_file() {
-                        match std::fs::copy(&s, &d) {
+                        let res = if interactive && d.exists() {
-                            Ok(_) => {
+                            interactive_copy_file(interactive, s, d, span)
-                                let msg = format!("copied {:} to {:}", &s.display(), &d.display());
+                        } else {
-                                result.push(Value::String { val: msg, span });
+                            copy_file(s, d, span)
-                            }
+                        };
-                            Err(e) => {
+                        result.push(res);
-                                let msg = "Can not copy source".to_string();
-                                let error = Value::Error {
-                                    error: ShellError::GenericError(
-                                        msg,
-                                        e.to_string(),
-                                        Some(span),
-                                        None,
-                                        Vec::new(),
-                                    ),
-                                };
-                                result.push(error);
-                            }
-                        }
                     }
                 }
             }
@@ -457,3 +434,42 @@ impl Command for Cp {
     //     Ok(PipelineData::new(call.head))
     // }
 }
+
+fn interactive_copy_file(interactive: bool, src: PathBuf, dst: PathBuf, span: Span) -> Value {
+    let (interaction, confirmed) =
+        try_interaction(interactive, "cp: overwrite", &dst.to_string_lossy());
+    if let Err(e) = interaction {
+        Value::Error {
+            error: ShellError::GenericError(
+                e.to_string(),
+                e.to_string(),
+                Some(span),
+                None,
+                Vec::new(),
+            ),
+        }
+    } else if !confirmed {
+        let msg = format!("{:} not copied to {:}", src.display(), dst.display());
+        Value::String { val: msg, span }
+    } else {
+        copy_file(src, dst, span)
+    }
+}
+
+fn copy_file(src: PathBuf, dst: PathBuf, span: Span) -> Value {
+    match std::fs::copy(&src, &dst) {
+        Ok(_) => {
+            let msg = format!("copied {:} to {:}", src.display(), dst.display());
+            Value::String { val: msg, span }
+        }
+        Err(e) => Value::Error {
+            error: ShellError::GenericError(
+                e.to_string(),
+                e.to_string(),
+                Some(span),
+                None,
+                Vec::new(),
+            ),
+        },
+    }
+}

crates/nu-command/src/filesystem/mv.rs

@@ -1,6 +1,6 @@
 use std::path::{Path, PathBuf};
 
-// use super::util::get_interactive_confirmation;
+use super::util::try_interaction;
 use nu_engine::env::current_dir;
 use nu_engine::CallExt;
 use nu_protocol::ast::Call;
@@ -50,7 +50,7 @@ impl Command for Mv {
                 "make mv to be verbose, showing files been moved.",
                 Some('v'),
             )
-            // .switch("interactive", "ask user to confirm action", Some('i'))
+            .switch("interactive", "ask user to confirm action", Some('i'))
             // .switch("force", "suppress error when no file", Some('f'))
             .category(Category::FileSystem)
     }
@@ -66,7 +66,7 @@ impl Command for Mv {
         let spanned_source: Spanned<String> = call.req(engine_state, stack, 0)?;
         let spanned_destination: Spanned<String> = call.req(engine_state, stack, 1)?;
         let verbose = call.has_flag("verbose");
-        // let interactive = call.has_flag("interactive");
+        let interactive = call.has_flag("interactive");
         // let force = call.has_flag("force");
 
         let ctrlc = engine_state.ctrlc.clone();
@@ -149,15 +149,24 @@ impl Command for Mv {
                         item: destination.clone(),
                         span: spanned_destination.span,
                     },
+                    interactive,
                 );
                 if let Err(error) = result {
                     Some(Value::Error { error })
                 } else if verbose {
-                    let val = format!(
+                    let val = if result.expect("Error value when unwrapping mv result") {
-                        "moved {:} to {:}",
+                        format!(
-                        entry.to_string_lossy(),
+                            "moved {:} to {:}",
-                        destination.to_string_lossy()
+                            entry.to_string_lossy(),
-                    );
+                            destination.to_string_lossy()
+                        )
+                    } else {
+                        format!(
+                            "{:} not moved to {:}",
+                            entry.to_string_lossy(),
+                            destination.to_string_lossy()
+                        )
+                    };
                     Some(Value::String { val, span })
                 } else {
                     None
@@ -190,7 +199,8 @@ impl Command for Mv {
 fn move_file(
     spanned_from: Spanned<PathBuf>,
     spanned_to: Spanned<PathBuf>,
-) -> Result<(), ShellError> {
+    interactive: bool,
+) -> Result<bool, ShellError> {
     let Spanned {
         item: from,
         span: from_span,
@@ -229,7 +239,26 @@ fn move_file(
         to.push(from_file_name);
     }
 
-    move_item(&from, from_span, &to)
+    if interactive && to.exists() {
+        let (interaction, confirmed) =
+            try_interaction(interactive, "mv: overwrite", &to.to_string_lossy());
+        if let Err(e) = interaction {
+            return Err(ShellError::GenericError(
+                format!("Error during interaction: {:}", e),
+                "could not move".into(),
+                None,
+                None,
+                Vec::new(),
+            ));
+        } else if !confirmed {
+            return Ok(false);
+        }
+    }
+
+    match move_item(&from, from_span, &to) {
+        Ok(()) => Ok(true),
+        Err(e) => Err(e),
+    }
 }
 
 fn move_item(from: &Path, from_span: Span, to: &Path) -> Result<(), ShellError> {

crates/nu-command/src/filesystem/rm.rs

@@ -9,7 +9,7 @@ use std::io::ErrorKind;
 use std::os::unix::prelude::FileTypeExt;
 use std::path::PathBuf;
 
-// use super::util::get_interactive_confirmation;
+use super::util::try_interaction;
 
 use nu_engine::env::current_dir;
 use nu_engine::CallExt;
@@ -67,7 +67,7 @@ impl Command for Rm {
                 "make rm to be verbose, showing files been deleted",
                 Some('v'),
             )
-            // .switch("interactive", "ask user to confirm action", Some('i'))
+            .switch("interactive", "ask user to confirm action", Some('i'))
             .rest(
                 "rest",
                 SyntaxShape::GlobPattern,
@@ -134,7 +134,7 @@ fn rm(
     let recursive = call.has_flag("recursive");
     let force = call.has_flag("force");
     let verbose = call.has_flag("verbose");
-    // let interactive = call.has_flag("interactive");
+    let interactive = call.has_flag("interactive");
 
     let ctrlc = engine_state.ctrlc.clone();
 
@@ -289,6 +289,9 @@ fn rm(
                     || is_fifo
                     || is_empty()
                 {
+                    let (interaction, confirmed) =
+                        try_interaction(interactive, "rm: remove", &f.to_string_lossy());
+
                     let result;
                     #[cfg(all(
                         feature = "trash-support",
@@ -297,9 +300,14 @@ fn rm(
                     ))]
                     {
                         use std::io::Error;
-                        result = if trash || (rm_always_trash && !permanent) {
+                        result = if let Err(e) = interaction {
+                            let e = Error::new(ErrorKind::Other, &*e.to_string());
+                            Err(e)
+                        } else if interactive && !confirmed {
+                            Ok(())
+                        } else if trash || (rm_always_trash && !permanent) {
                             trash::delete(&f).map_err(|e: trash::Error| {
-                                Error::new(ErrorKind::Other, format!("{:?}", e))
+                                Error::new(ErrorKind::Other, format!("{:?}\nTry '--trash' flag", e))
                             })
                         } else if metadata.is_file() || is_socket || is_fifo {
                             std::fs::remove_file(&f)
@@ -313,7 +321,13 @@ fn rm(
                         target_os = "ios"
                     ))]
                     {
-                        result = if metadata.is_file() || is_socket || is_fifo {
+                        use std::io::{Error, ErrorKind};
+                        result = if let Err(e) = interaction {
+                            let e = Error::new(ErrorKind::Other, &*e.to_string());
+                            Err(e)
+                        } else if interactive && !confirmed {
+                            Ok(())
+                        } else if metadata.is_file() || is_socket || is_fifo {
                             std::fs::remove_file(&f)
                         } else {
                             std::fs::remove_dir_all(&f)
@@ -321,7 +335,7 @@ fn rm(
                     }
 
                     if let Err(e) = result {
-                        let msg = format!("Could not delete because: {:}\nTry '--trash' flag", e);
+                        let msg = format!("Could not delete because: {:}", e);
                         Value::Error {
                             error: ShellError::GenericError(
                                 msg,
@@ -332,7 +346,12 @@ fn rm(
                             ),
                         }
                     } else if verbose {
-                        let val = format!("deleted {:}", f.to_string_lossy());
+                        let msg = if interactive && !confirmed {
+                            "not deleted"
+                        } else {
+                            "deleted"
+                        };
+                        let val = format!("{} {:}", msg, f.to_string_lossy());
                         Value::String { val, span }
                     } else {
                         Value::Nothing { span }

crates/nu-command/src/filesystem/util.rs

@@ -97,8 +97,31 @@ pub struct Resource {
 
 impl Resource {}
 
+pub fn try_interaction(
+    interactive: bool,
+    prompt_msg: &str,
+    file_name: &str,
+) -> (Result<Option<bool>, Box<dyn Error>>, bool) {
+    let interaction = if interactive {
+        let prompt = format!("{} '{}'? ", prompt_msg, file_name);
+        match get_interactive_confirmation(prompt) {
+            Ok(i) => Ok(Some(i)),
+            Err(e) => Err(e),
+        }
+    } else {
+        Ok(None)
+    };
+
+    let confirmed = match interaction {
+        Ok(maybe_input) => maybe_input.unwrap_or(false),
+        Err(_) => false,
+    };
+
+    (interaction, confirmed)
+}
+
 #[allow(dead_code)]
-pub fn get_interactive_confirmation(prompt: String) -> Result<bool, Box<dyn Error>> {
+fn get_interactive_confirmation(prompt: String) -> Result<bool, Box<dyn Error>> {
     let input = Input::new()
         .with_prompt(prompt)
         .validate_with(|c_input: &String| -> Result<(), String> {