diff --git a/crates/nu-command/src/filesystem/rm.rs b/crates/nu-command/src/filesystem/rm.rs index 74c7c219f3..41f48e92d1 100644 --- a/crates/nu-command/src/filesystem/rm.rs +++ b/crates/nu-command/src/filesystem/rm.rs @@ -419,15 +419,9 @@ fn rm( } if let Err(e) = result { - let msg = format!("Could not delete because: {e:}"); + let msg = format!("Could not delete {:}: {e:}", f.to_string_lossy()); Value::Error { - error: Box::new(ShellError::GenericError( - msg, - e.to_string(), - Some(span), - None, - Vec::new(), - )), + error: Box::new(ShellError::RemoveNotPossible(msg, span)), } } else if verbose { let msg = if interactive && !confirmed { diff --git a/crates/nu-command/tests/commands/rm.rs b/crates/nu-command/tests/commands/rm.rs index cf19301056..f02613086f 100644 --- a/crates/nu-command/tests/commands/rm.rs +++ b/crates/nu-command/tests/commands/rm.rs @@ -1,6 +1,8 @@ use nu_test_support::fs::{files_exist_at, Stub::EmptyFile}; use nu_test_support::nu; use nu_test_support::playground::Playground; +use std::fs; +use std::path::PathBuf; #[test] fn removes_a_file() { @@ -349,3 +351,60 @@ fn remove_ignores_ansi() { assert_eq!(actual.out, "true"); }); } + +struct Cleanup<'a> { + dir_to_clean: &'a PathBuf, +} + +fn set_dir_read_only(directory: &PathBuf, read_only: bool) { + let mut permissions = fs::metadata(directory).unwrap().permissions(); + permissions.set_readonly(read_only); + fs::set_permissions(directory, permissions).expect("failed to set directory permissions"); +} + +impl<'a> Drop for Cleanup<'a> { + /// Restores write permissions to the given directory so that the Playground can be successfully + /// cleaned up. + fn drop(&mut self) { + set_dir_read_only(self.dir_to_clean, false); + } +} + +#[test] +// This test is only about verifying file names are included in rm error messages. It is easier +// to only have this work on non-windows systems (i.e., unix-like) than to try to get the +// permissions to work on all platforms. +#[cfg(not(windows))] +fn rm_prints_filenames_on_error() { + Playground::setup("rm_prints_filenames_on_error", |dirs, sandbox| { + let file_names = vec!["test1.txt", "test2.txt"]; + + let with_files = file_names + .iter() + .map(|file_name| EmptyFile(file_name)) + .collect(); + sandbox.with_files(with_files); + + let test_dir = dirs.test(); + + set_dir_read_only(test_dir, true); + let _cleanup = Cleanup { + dir_to_clean: &test_dir, + }; + + // This rm is expected to fail, and stderr output indicating so is also expected. + let actual = nu!(cwd: test_dir, "rm test*.txt"); + + assert!(files_exist_at(file_names.clone(), test_dir)); + for file_name in file_names { + let path = test_dir.join(file_name); + let substr = format!("Could not delete {}", path.to_string_lossy()); + assert!( + actual.err.contains(&substr), + "Matching: {}\n=== Command stderr:\n{}\n=== End stderr", + substr, + actual.err + ); + } + }); +} diff --git a/crates/nu-protocol/src/shell_error.rs b/crates/nu-protocol/src/shell_error.rs index 12a5312078..210fe80788 100644 --- a/crates/nu-protocol/src/shell_error.rs +++ b/crates/nu-protocol/src/shell_error.rs @@ -869,9 +869,12 @@ pub enum ShellError { ChangeModifiedTimeNotPossible(String, #[label("{0}")] Span), /// Unable to remove this item. + /// + /// ## Resolution + /// + /// Removal can fail for a number of reasons, such as permissions problems. Refer to the specific error message for more details. #[error("Remove not possible")] #[diagnostic(code(nu::shell::remove_not_possible))] - // NOTE: Currently unused. Remove? RemoveNotPossible(String, #[label("{0}")] Span), // These three are unused. Remove?