From 2c58beec1329eedfb9aa126520b061671cdadbeb Mon Sep 17 00:00:00 2001 From: njbull4 Date: Wed, 18 May 2022 07:53:46 -0700 Subject: [PATCH] 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 | 98 ++++++++++++++---------- crates/nu-command/src/filesystem/mv.rs | 49 +++++++++--- crates/nu-command/src/filesystem/rm.rs | 35 +++++++-- crates/nu-command/src/filesystem/util.rs | 25 +++++- 4 files changed, 147 insertions(+), 60 deletions(-) diff --git a/crates/nu-command/src/filesystem/cp.rs b/crates/nu-command/src/filesystem/cp.rs index 65c0bcee44..8684ca014f 100644 --- a/crates/nu-command/src/filesystem/cp.rs +++ b/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, - SyntaxShape, Value, + Category, Example, IntoInterruptiblePipelineData, PipelineData, ShellError, Signature, Span, + 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 = 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) { - Ok(_) => { - let msg = - format!("copied {:} to {:}", src.display(), dst.display()); - result.push(Value::String { val: msg, span }); - } - Err(e) => { - let error = Value::Error { - error: ShellError::GenericError( - e.to_string(), - e.to_string(), - Some(span), - None, - Vec::new(), - ), - }; - result.push(error); - } - } + let res = if interactive && dst.exists() { + interactive_copy_file(interactive, src, dst, span) + } else { + copy_file(src, dst, span) + }; + result.push(res); } } } else if entry.is_dir() { @@ -222,25 +212,12 @@ impl Command for Cp { } if s.is_file() { - match std::fs::copy(&s, &d) { - Ok(_) => { - let msg = format!("copied {:} to {:}", &s.display(), &d.display()); - result.push(Value::String { val: msg, span }); - } - Err(e) => { - 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); - } - } + let res = if interactive && d.exists() { + interactive_copy_file(interactive, s, d, span) + } else { + copy_file(s, d, span) + }; + result.push(res); } } } @@ -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(), + ), + }, + } +} diff --git a/crates/nu-command/src/filesystem/mv.rs b/crates/nu-command/src/filesystem/mv.rs index 6b33b56ceb..3672a71c78 100644 --- a/crates/nu-command/src/filesystem/mv.rs +++ b/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 = call.req(engine_state, stack, 0)?; let spanned_destination: Spanned = 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!( - "moved {:} to {:}", - entry.to_string_lossy(), - destination.to_string_lossy() - ); + let val = if result.expect("Error value when unwrapping mv result") { + format!( + "moved {:} to {:}", + 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, spanned_to: Spanned, -) -> Result<(), ShellError> { + interactive: bool, +) -> Result { 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> { diff --git a/crates/nu-command/src/filesystem/rm.rs b/crates/nu-command/src/filesystem/rm.rs index 825fc1d5be..4a46046bb5 100644 --- a/crates/nu-command/src/filesystem/rm.rs +++ b/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 } diff --git a/crates/nu-command/src/filesystem/util.rs b/crates/nu-command/src/filesystem/util.rs index 0ee86d0e30..c58a823b5a 100644 --- a/crates/nu-command/src/filesystem/util.rs +++ b/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, Box>, 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> { +fn get_interactive_confirmation(prompt: String) -> Result> { let input = Input::new() .with_prompt(prompt) .validate_with(|c_input: &String| -> Result<(), String> {