diff --git a/crates/nu-command/src/filesystem/mv.rs b/crates/nu-command/src/filesystem/mv.rs index c2daa6e823..8d18fe5bba 100644 --- a/crates/nu-command/src/filesystem/mv.rs +++ b/crates/nu-command/src/filesystem/mv.rs @@ -50,8 +50,8 @@ impl Command for Mv { "make mv to be verbose, showing files been moved.", Some('v'), ) + .switch("force", "overwrite the destination.", Some('f')) .switch("interactive", "ask user to confirm action", Some('i')) - // .switch("force", "suppress error when no file", Some('f')) .category(Category::FileSystem) } @@ -76,7 +76,7 @@ impl Command for Mv { let spanned_destination: Spanned = call.req(engine_state, stack, 1)?; let verbose = call.has_flag("verbose"); let interactive = call.has_flag("interactive"); - // let force = call.has_flag("force"); + let force = call.has_flag("force"); let ctrlc = engine_state.ctrlc.clone(); @@ -101,12 +101,22 @@ impl Command for Mv { // // First, the destination exists. // - If a directory, move everything into that directory, otherwise - // - if only a single source, overwrite the file, otherwise - // - error. + // - if only a single source, and --force (or -f) is provided overwrite the file, + // - otherwise error. // // Second, the destination doesn't exist, so we can only rename a single source. Otherwise // it's an error. + if destination.exists() && !force && !destination.is_dir() && !source.is_dir() { + return Err(ShellError::GenericError( + "Destination file already exists".into(), + "you can use -f, --force to force overwriting the destination".into(), + Some(spanned_destination.span), + None, + Vec::new(), + )); + } + if (destination.exists() && !destination.is_dir() && sources.len() > 1) || (!destination.exists() && sources.len() > 1) { diff --git a/crates/nu-command/tests/commands/move_/mv.rs b/crates/nu-command/tests/commands/move_/mv.rs index 51f57db64d..2f585e7215 100644 --- a/crates/nu-command/tests/commands/move_/mv.rs +++ b/crates/nu-command/tests/commands/move_/mv.rs @@ -23,7 +23,7 @@ fn moves_a_file() { } #[test] -fn overwrites_if_moving_to_existing_file() { +fn overwrites_if_moving_to_existing_file_and_force_provided() { Playground::setup("mv_test_2", |dirs, sandbox| { sandbox.with_files(vec![EmptyFile("andres.txt"), EmptyFile("jonathan.txt")]); @@ -32,7 +32,7 @@ fn overwrites_if_moving_to_existing_file() { nu!( cwd: dirs.test(), - "mv andres.txt jonathan.txt" + "mv andres.txt -f jonathan.txt" ); assert!(!original.exists()); @@ -207,6 +207,19 @@ fn errors_if_source_doesnt_exist() { }) } +#[test] +fn error_if_moving_to_existing_file_without_force() { + Playground::setup("mv_test_10_0", |dirs, sandbox| { + sandbox.with_files(vec![EmptyFile("andres.txt"), EmptyFile("jonathan.txt")]); + + let actual = nu!( + cwd: dirs.test(), + "mv andres.txt jonathan.txt" + ); + assert!(actual.err.contains("file already exists")) + }) +} + #[test] fn errors_if_destination_doesnt_exist() { Playground::setup("mv_test_10_1", |dirs, sandbox| { @@ -438,7 +451,7 @@ fn mv_change_case_of_file() { nu!( cwd: dirs.test(), - format!("mv {original_file_name} {new_file_name}") + format!("mv {original_file_name} -f {new_file_name}") ); // Doing this instead of `Path::exists()` because we need to check file existence in