diff --git a/crates/nu-cli/src/commands/rm.rs b/crates/nu-cli/src/commands/rm.rs index 10a2c44e5e..2c766f0374 100644 --- a/crates/nu-cli/src/commands/rm.rs +++ b/crates/nu-cli/src/commands/rm.rs @@ -10,7 +10,7 @@ pub struct Remove; #[derive(Deserialize)] pub struct RemoveArgs { - pub target: Tagged, + pub rest: Vec>, pub recursive: Tagged, pub trash: Tagged, } @@ -22,17 +22,17 @@ impl PerItemCommand for Remove { fn signature(&self) -> Signature { Signature::build("rm") - .required("path", SyntaxShape::Pattern, "the file path to remove") .switch( "trash", "use the platform's recycle bin instead of permanently deleting", Some('t'), ) .switch("recursive", "delete subdirectories recursively", Some('r')) + .rest(SyntaxShape::Pattern, "the file path(s) to remove") } fn usage(&self) -> &str { - "Remove a file" + "Remove file(s)" } fn run( diff --git a/crates/nu-cli/src/shell/filesystem_shell.rs b/crates/nu-cli/src/shell/filesystem_shell.rs index 1554c5f7a5..fe9143b548 100644 --- a/crates/nu-cli/src/shell/filesystem_shell.rs +++ b/crates/nu-cli/src/shell/filesystem_shell.rs @@ -14,6 +14,7 @@ use nu_parser::ExpandContext; use nu_protocol::{Primitive, ReturnSuccess, UntaggedValue}; use rustyline::completion::FilenameCompleter; use rustyline::hint::{Hinter, HistoryHinter}; +use std::collections::HashMap; use std::path::{Component, Path, PathBuf}; use std::sync::atomic::Ordering; use trash as SendToTrash; @@ -991,7 +992,7 @@ impl Shell for FilesystemShell { fn rm( &self, RemoveArgs { - target, + rest: targets, recursive, trash, }: RemoveArgs, @@ -1000,125 +1001,141 @@ impl Shell for FilesystemShell { ) -> Result { let name_tag = name; - if target.item.to_str() == Some(".") || target.item.to_str() == Some("..") { + if targets.is_empty() { return Err(ShellError::labeled_error( - "Remove aborted. \".\" or \"..\" may not be removed.", - "\".\" or \"..\" may not be removed", - target.tag, + "rm requires target paths", + "needs parameter", + name_tag, )); } - let mut path = PathBuf::from(path); + let mut all_targets: HashMap = HashMap::new(); + for target in targets { + if target.item.to_str() == Some(".") || target.item.to_str() == Some("..") { + return Err(ShellError::labeled_error( + "Remove aborted. \".\" or \"..\" may not be removed.", + "\".\" or \"..\" may not be removed", + target.tag, + )); + } - path.push(&target.item); - - match glob::glob(&path.to_string_lossy()) { - Ok(files) => { - let files: Vec<_> = files.collect(); - if files.is_empty() { - Err(ShellError::labeled_error( - "Remove aborted. Not a valid path", - "not a valid path", - target.tag, + let mut path = PathBuf::from(path); + path.push(&target.item); + match glob::glob(&path.to_string_lossy()) { + Ok(files) => { + for file in files { + match file { + Ok(ref f) => { + all_targets + .entry(f.clone()) + .or_insert_with(|| target.tag.clone()); + } + Err(e) => { + let msg = format!("Could not remove {:}", path.to_string_lossy()); + return Err(ShellError::labeled_error( + msg, + e.to_string(), + &target.tag, + )); + } + } + } + } + Err(e) => { + return Err(ShellError::labeled_error( + format!("Remove aborted. {:}", e.to_string()), + e.to_string(), + &name_tag, )) - } else { - let stream = async_stream! { - for file in files.iter() { - match file { - Ok(f) => { - let is_empty = match f.read_dir() { - Ok(mut p) => p.next().is_none(), - Err(_) => false - }; + } + }; + } - let valid_target = - f.exists() && (!f.is_dir() || (is_empty || recursive.item)); - if valid_target { - if trash.item { - match SendToTrash::remove(f) { - Err(e) => { - let msg = format!( - "Could not delete {:}", - f.to_string_lossy() - ); - let label = format!("{:?}", e); - yield Err(ShellError::labeled_error( - msg, - label, - &target.tag, - )) - }, - Ok(()) => { - let val = format!("deleted {:}", f.to_string_lossy()).into(); - yield Ok(ReturnSuccess::Value(val)) - }, - } - } else { - let success = if f.is_dir() { - std::fs::remove_dir_all(f) - } else { - std::fs::remove_file(f) - }; - match success { - Err(e) => { - let msg = format!( - "Could not delete {:}", - f.to_string_lossy() - ); - yield Err(ShellError::labeled_error( - msg, - e.to_string(), - &target.tag, - )) - }, - Ok(()) => { - let val = format!("deleted {:}", f.to_string_lossy()).into(); - yield Ok(ReturnSuccess::Value( - val, - )) - }, - } - } - } else { - if f.is_dir() { - let msg = format!( - "Cannot remove {:}. try --recursive", - f.to_string_lossy() - ); - yield Err(ShellError::labeled_error( - msg, - "cannot remove non-empty directory", - &target.tag, - )) - } else { - let msg = format!("Invalid file: {:}", f.to_string_lossy()); - yield Err(ShellError::labeled_error( - msg, - "invalid file", - &target.tag, - )) - } - } - } + if all_targets.is_empty() { + Err(ShellError::labeled_error( + "Remove aborted. No valid paths", + "no valid paths", + name_tag, + )) + } else { + let stream = async_stream! { + for (f, tag) in all_targets.iter() { + let is_empty = match f.read_dir() { + Ok(mut p) => p.next().is_none(), + Err(_) => false + }; + + let valid_target = + f.exists() && (!f.is_dir() || (is_empty || recursive.item)); + if valid_target { + if trash.item { + match SendToTrash::remove(f) { Err(e) => { - let msg = format!("Could not remove {:}", path.to_string_lossy()); + let msg = format!( + "Could not delete {:}", + f.to_string_lossy() + ); + let label = format!("{:?}", e); + yield Err(ShellError::labeled_error( + msg, + label, + tag, + )) + }, + Ok(()) => { + let val = format!("deleted {:}", f.to_string_lossy()).into(); + yield Ok(ReturnSuccess::Value(val)) + }, + } + } else { + let success = if f.is_dir() { + std::fs::remove_dir_all(f) + } else { + std::fs::remove_file(f) + }; + match success { + Err(e) => { + let msg = format!( + "Could not delete {:}", + f.to_string_lossy() + ); yield Err(ShellError::labeled_error( msg, e.to_string(), - &target.tag, + tag, + )) + }, + Ok(()) => { + let val = format!("deleted {:}", f.to_string_lossy()).into(); + yield Ok(ReturnSuccess::Value( + val, )) }, } - } - }; - Ok(stream.to_output_stream()) + } + } else { + if f.is_dir() { + let msg = format!( + "Cannot remove {:}. try --recursive", + f.to_string_lossy() + ); + yield Err(ShellError::labeled_error( + msg, + "cannot remove non-empty directory", + tag, + )) + } else { + let msg = format!("Invalid file: {:}", f.to_string_lossy()); + yield Err(ShellError::labeled_error( + msg, + "invalid file", + tag, + )) + } + } } - } - Err(e) => Err(ShellError::labeled_error( - format!("Remove aborted. {:}", e.to_string()), - e.to_string(), - &name_tag, - )), + }; + Ok(stream.to_output_stream()) } } diff --git a/crates/nu-cli/tests/commands/rm.rs b/crates/nu-cli/tests/commands/rm.rs index caaee55e83..14b7962b79 100644 --- a/crates/nu-cli/tests/commands/rm.rs +++ b/crates/nu-cli/tests/commands/rm.rs @@ -159,3 +159,86 @@ fn errors_if_attempting_to_delete_two_dot_as_argument() { assert!(actual.contains("may not be removed")); }) } + +#[test] +fn removes_multiple_directories() { + Playground::setup("rm_test_9", |dirs, sandbox| { + sandbox + .within("src") + .with_files(vec![EmptyFile("a.rs"), EmptyFile("b.rs")]) + .within("src/cli") + .with_files(vec![EmptyFile("c.rs"), EmptyFile("d.rs")]) + .within("test") + .with_files(vec![EmptyFile("a_test.rs"), EmptyFile("b_test.rs")]); + + nu!( + cwd: dirs.test(), + "rm src test --recursive" + ); + + assert_eq!( + Playground::glob_vec(&format!("{}/*", dirs.test().display())), + Vec::::new() + ); + }) +} + +#[test] +fn removes_multiple_files() { + Playground::setup("rm_test_10", |dirs, sandbox| { + sandbox.with_files(vec![ + EmptyFile("yehuda.txt"), + EmptyFile("jonathan.txt"), + EmptyFile("andres.txt"), + ]); + + nu!( + cwd: dirs.test(), + "rm yehuda.txt jonathan.txt andres.txt" + ); + + assert_eq!( + Playground::glob_vec(&format!("{}/*", dirs.test().display())), + Vec::::new() + ); + }) +} + +#[test] +fn removes_multiple_files_with_asterisks() { + Playground::setup("rm_test_11", |dirs, sandbox| { + sandbox.with_files(vec![ + EmptyFile("yehuda.txt"), + EmptyFile("jonathan.txt"), + EmptyFile("andres.toml"), + ]); + + nu!( + cwd: dirs.test(), + "rm *.txt *.toml" + ); + + assert_eq!( + Playground::glob_vec(&format!("{}/*", dirs.test().display())), + Vec::::new() + ); + }) +} + +#[test] +fn allows_doubly_specified_file() { + Playground::setup("rm_test_12", |dirs, sandbox| { + sandbox.with_files(vec![EmptyFile("yehuda.txt"), EmptyFile("jonathan.toml")]); + + let actual = nu!( + cwd: dirs.test(), + "rm *.txt yehuda* *.toml" + ); + + assert_eq!( + Playground::glob_vec(&format!("{}/*", dirs.test().display())), + Vec::::new() + ); + assert!(!actual.contains("error")) + }) +}