mv: gnu test case mv-n compatibility

src/uu/cp/src/cp.rs

@@ -38,7 +38,7 @@ use uucore::{backup_control, update_control};
 pub use uucore::{backup_control::BackupMode, update_control::UpdateMode};
 use uucore::{
     format_usage, help_about, help_section, help_usage, prompt_yes,
-    shortcut_value_parser::ShortcutValueParser, show_error, show_warning, util_name,
+    shortcut_value_parser::ShortcutValueParser, show_error, show_warning,
 };
 
 use crate::copydir::copy_directory;
@@ -78,8 +78,10 @@ quick_error! {
         StripPrefixError(err: StripPrefixError) { from() }
 
         /// Result of a skipped file
-        /// Currently happens when "no" is selected in interactive mode
-        Skipped { }
+        /// Currently happens when "no" is selected in interactive mode or when
+        /// `no-clobber` flag is set and destination is already present.
+        /// `exit with error` is used to determine which exit code should be returned.
+        Skipped(exit_with_error:bool) { }
 
         /// Result of a skipped file
         InvalidArgument(description: String) { display("{}", description) }
@@ -1207,7 +1209,7 @@ fn show_error_if_needed(error: &Error) {
         Error::NotAllFilesCopied => {
             // Need to return an error code
         }
-        Error::Skipped => {
+        Error::Skipped(_) => {
             // touch a b && echo "n"|cp -i a b && echo $?
             // should return an error from GNU 9.2
         }
@@ -1292,7 +1294,9 @@ pub fn copy(sources: &[PathBuf], target: &Path, options: &Options) -> CopyResult
                 &mut copied_files,
             ) {
                 show_error_if_needed(&error);
-                non_fatal_errors = true;
+                if !matches!(error, Error::Skipped(false)) {
+                    non_fatal_errors = true;
+                }
             } else {
                 copied_destinations.insert(dest.clone());
             }
@@ -1394,17 +1398,19 @@ fn copy_source(
 }
 
 impl OverwriteMode {
-    fn verify(&self, path: &Path) -> CopyResult<()> {
+    fn verify(&self, path: &Path, debug: bool) -> CopyResult<()> {
         match *self {
             Self::NoClobber => {
-                eprintln!("{}: not replacing {}", util_name(), path.quote());
-                Err(Error::NotAllFilesCopied)
+                if debug {
+                    println!("skipped {}", path.quote());
+                }
+                Err(Error::Skipped(false))
             }
             Self::Interactive(_) => {
                 if prompt_yes!("overwrite {}?", path.quote()) {
                     Ok(())
                 } else {
-                    Err(Error::Skipped)
+                    Err(Error::Skipped(true))
                 }
             }
             Self::Clobber(_) => Ok(()),
@@ -1651,7 +1657,7 @@ fn handle_existing_dest(
     }
 
     if options.update != UpdateMode::ReplaceIfOlder {
-        options.overwrite.verify(dest)?;
+        options.overwrite.verify(dest, options.debug)?;
     }
 
     let mut is_dest_removed = false;
@@ -1889,6 +1895,9 @@ fn handle_copy_mode(
 
                         return Ok(());
                     }
+                    update_control::UpdateMode::ReplaceNoneFail => {
+                        return Err(Error::Error(format!("not replacing '{}'", dest.display())));
+                    }
                     update_control::UpdateMode::ReplaceIfOlder => {
                         let dest_metadata = fs::symlink_metadata(dest)?;
 
@@ -1897,7 +1906,7 @@ fn handle_copy_mode(
                         if src_time <= dest_time {
                             return Ok(());
                         } else {
-                            options.overwrite.verify(dest)?;
+                            options.overwrite.verify(dest, options.debug)?;
 
                             copy_helper(
                                 source,
@@ -2259,7 +2268,7 @@ fn copy_helper(
         File::create(dest).context(dest.display().to_string())?;
     } else if source_is_fifo && options.recursive && !options.copy_contents {
         #[cfg(unix)]
-        copy_fifo(dest, options.overwrite)?;
+        copy_fifo(dest, options.overwrite, options.debug)?;
     } else if source_is_symlink {
         copy_link(source, dest, symlinked_files)?;
     } else {
@@ -2284,9 +2293,9 @@ fn copy_helper(
 // "Copies" a FIFO by creating a new one. This workaround is because Rust's
 // built-in fs::copy does not handle FIFOs (see rust-lang/rust/issues/79390).
 #[cfg(unix)]
-fn copy_fifo(dest: &Path, overwrite: OverwriteMode) -> CopyResult<()> {
+fn copy_fifo(dest: &Path, overwrite: OverwriteMode, debug: bool) -> CopyResult<()> {
     if dest.exists() {
-        overwrite.verify(dest)?;
+        overwrite.verify(dest, debug)?;
         fs::remove_file(dest)?;
     }
 

src/uu/mv/src/mv.rs

@@ -83,6 +83,9 @@
 
     /// '-g, --progress'
     pub progress_bar: bool,
+
+    /// `--debug`
+    pub debug: bool,
 }
 
 /// specifies behavior of the overwrite flag
@@ -109,6 +112,7 @@
 static OPT_VERBOSE: &str = "verbose";
 static OPT_PROGRESS: &str = "progress";
 static ARG_FILES: &str = "files";
+static OPT_DEBUG: &str = "debug";
 
 #[uucore::main]
 pub fn uumain(args: impl uucore::Args) -> UResult<()> {
@@ -135,10 +139,14 @@
     let backup_mode = backup_control::determine_backup_mode(&matches)?;
     let update_mode = update_control::determine_update_mode(&matches);
 
-    if overwrite_mode == OverwriteMode::NoClobber && backup_mode != BackupMode::NoBackup {
+    if backup_mode != BackupMode::NoBackup
+        && (overwrite_mode == OverwriteMode::NoClobber
+            || update_mode == UpdateMode::ReplaceNone
+            || update_mode == UpdateMode::ReplaceNoneFail)
+    {
         return Err(UUsageError::new(
             1,
-            "options --backup and --no-clobber are mutually exclusive",
+            "cannot combine --backup with -n/--no-clobber or --update=none-fail",
         ));
     }
 
@@ -161,9 +169,10 @@
         update: update_mode,
         target_dir,
         no_target_dir: matches.get_flag(OPT_NO_TARGET_DIRECTORY),
-        verbose: matches.get_flag(OPT_VERBOSE),
+        verbose: matches.get_flag(OPT_VERBOSE) || matches.get_flag(OPT_DEBUG),
         strip_slashes: matches.get_flag(OPT_STRIP_TRAILING_SLASHES),
         progress_bar: matches.get_flag(OPT_PROGRESS),
+        debug: matches.get_flag(OPT_DEBUG),
     };
 
     mv(&files[..], &opts)
@@ -256,6 +265,12 @@
                 .value_parser(ValueParser::os_string())
                 .value_hint(clap::ValueHint::AnyPath),
         )
+        .arg(
+            Arg::new(OPT_DEBUG)
+                .long(OPT_DEBUG)
+                .help("explain how a file is copied. Implies -v")
+                .action(ArgAction::SetTrue),
+        )
 }
 
 fn determine_overwrite_mode(matches: &ArgMatches) -> OverwriteMode {
@@ -521,6 +536,9 @@
         }
 
         if opts.update == UpdateMode::ReplaceNone {
+            if opts.debug {
+                println!("skipped {}", to.quote());
+            }
             return Ok(());
         }
 
@@ -530,10 +548,17 @@
             return Ok(());
         }
 
+        if opts.update == UpdateMode::ReplaceNoneFail {
+            let err_msg = format!("not replacing {}", to.quote());
+            return Err(io::Error::new(io::ErrorKind::Other, err_msg));
+        }
+
         match opts.overwrite {
             OverwriteMode::NoClobber => {
-                let err_msg = format!("not replacing {}", to.quote());
-                return Err(io::Error::new(io::ErrorKind::Other, err_msg));
+                if opts.debug {
+                    println!("skipped {}", to.quote());
+                }
+                return Ok(());
             }
             OverwriteMode::Interactive => {
                 if !prompt_yes!("overwrite {}?", to.quote()) {

src/uucore/src/lib/features/update_control.rs

@@ -59,6 +59,7 @@ pub enum UpdateMode {
     /// --update=`older`
     /// -u
     ReplaceIfOlder,
+    ReplaceNoneFail,
 }
 
 pub mod arguments {
@@ -76,7 +77,7 @@ pub mod arguments {
         clap::Arg::new(OPT_UPDATE)
             .long("update")
             .help("move only when the SOURCE file is newer than the destination file or when the destination file is missing")
-            .value_parser(ShortcutValueParser::new(["none", "all", "older"]))
+            .value_parser(ShortcutValueParser::new(["none", "all", "older","none-fail"]))
             .num_args(0..=1)
             .default_missing_value("older")
             .require_equals(true)
@@ -130,6 +131,7 @@ pub fn determine_update_mode(matches: &ArgMatches) -> UpdateMode {
             "all" => UpdateMode::ReplaceAll,
             "none" => UpdateMode::ReplaceNone,
             "older" => UpdateMode::ReplaceIfOlder,
+            "none-fail" => UpdateMode::ReplaceNoneFail,
             _ => unreachable!("other args restricted by clap"),
         }
     } else if matches.get_flag(arguments::OPT_UPDATE_NO_ARG) {

src/uucore/src/lib/parser/shortcut_value_parser.rs

@@ -2,7 +2,7 @@
 //
 // For the full copyright and license information, please view the LICENSE
 // file that was distributed with this source code.
-// spell-checker:ignore abcdefgh abef
+// spell-checker:ignore abcdefgh abef Strs
 
 //! A parser that accepts shortcuts for values.
 //! `ShortcutValueParser` is similar to clap's `PossibleValuesParser`
@@ -32,6 +32,7 @@
         cmd: &clap::Command,
         arg: Option<&clap::Arg>,
         value: &str,
+        possible_values: &[&PossibleValue],
     ) -> clap::Error {
         let mut err = clap::Error::new(ErrorKind::InvalidValue).with_cmd(cmd);
 
@@ -52,10 +53,39 @@
             ContextValue::Strings(self.0.iter().map(|x| x.get_name().to_string()).collect()),
         );
 
+        // if `possible_values` is not empty then that means this error is because of an ambiguous value.
+        if !possible_values.is_empty() {
+            add_ambiguous_value_tip(possible_values, &mut err, value);
+        }
         err
     }
 }
 
+/// Adds a suggestion when error is because of ambiguous values based on the provided possible values.
+fn add_ambiguous_value_tip(
+    possible_values: &[&PossibleValue],
+    err: &mut clap::error::Error,
+    value: &str,
+) {
+    let mut formatted_possible_values = String::new();
+    for (i, s) in possible_values.iter().enumerate() {
+        formatted_possible_values.push_str(&format!("'{}'", s.get_name()));
+        if i < possible_values.len() - 2 {
+            formatted_possible_values.push_str(", ");
+        } else if i < possible_values.len() - 1 {
+            formatted_possible_values.push_str(" or ");
+        }
+    }
+    err.insert(
+        ContextKind::Suggested,
+        ContextValue::StyledStrs(vec![format!(
+            "It looks like '{}' could match several values. Did you mean {}?",
+            value, formatted_possible_values
+        )
+        .into()]),
+    );
+}
+
 impl TypedValueParser for ShortcutValueParser {
     type Value = String;
 
@@ -76,13 +106,13 @@
             .collect();
 
         match matched_values.len() {
-            0 => Err(self.generate_clap_error(cmd, arg, value)),
+            0 => Err(self.generate_clap_error(cmd, arg, value, &[])),
             1 => Ok(matched_values[0].get_name().to_string()),
             _ => {
                 if let Some(direct_match) = matched_values.iter().find(|x| x.get_name() == value) {
                     Ok(direct_match.get_name().to_string())
                 } else {
-                    Err(self.generate_clap_error(cmd, arg, value))
+                    Err(self.generate_clap_error(cmd, arg, value, &matched_values))
                 }
             }
         }
@@ -143,7 +173,11 @@
 
         for ambiguous_value in ambiguous_values {
             let result = parser.parse_ref(&cmd, None, OsStr::new(ambiguous_value));
-            assert_eq!(ErrorKind::InvalidValue, result.unwrap_err().kind());
+            assert_eq!(ErrorKind::InvalidValue, result.as_ref().unwrap_err().kind());
+            assert!(result.unwrap_err().to_string().contains(&format!(
+                "It looks like '{}' could match several values. Did you mean 'abcd' or 'abef'?",
+                ambiguous_value
+            )));
         }
 
         let result = parser.parse_ref(&cmd, None, OsStr::new("abc"));