Merge pull request #4148 from tertsdiepraam/prompt_yes

`uucore`: add `prompt_yes` macro

src/uu/cp/src/cp.rs

@@ -18,7 +18,7 @@ use std::env;
 #[cfg(not(windows))]
 use std::ffi::CString;
 use std::fs::{self, File, OpenOptions};
-use std::io::{self, stderr, stdin, Write};
+use std::io;
 #[cfg(unix)]
 use std::os::unix::ffi::OsStrExt;
 #[cfg(unix)]
@@ -39,7 +39,7 @@ use uucore::error::{set_exit_code, UClapError, UError, UResult, UUsageError};
 use uucore::fs::{
     canonicalize, paths_refer_to_same_file, FileInformation, MissingHandling, ResolveMode,
 };
-use uucore::{crash, crash_if_err, format_usage, show_error, show_warning};
+use uucore::{crash, format_usage, prompt_yes, show_error, show_warning};
 
 mod copydir;
 use crate::copydir::copy_directory;
@@ -102,24 +102,6 @@ impl UError for Error {
     }
 }
 
-/// Prompts the user yes/no and returns `true` if they successfully
-/// answered yes.
-macro_rules! prompt_yes(
-    ($($args:tt)+) => ({
-        eprint!($($args)+);
-        eprint!(" [y/N]: ");
-        crash_if_err!(1, stderr().flush());
-        let mut s = String::new();
-        match stdin().read_line(&mut s) {
-            Ok(_) => match s.char_indices().next() {
-                Some((_, x)) => x == 'y' || x == 'Y',
-                _ => false
-            },
-            _ => false
-        }
-    })
-);
-
 pub type CopyResult<T> = Result<T, Error>;
 pub type Source = PathBuf;
 pub type SourceSlice = Path;
@@ -1085,7 +1067,7 @@ impl OverwriteMode {
         match *self {
             Self::NoClobber => Err(Error::NotAllFilesCopied),
             Self::Interactive(_) => {
-                if prompt_yes!("{}: overwrite {}? ", uucore::util_name(), path.quote()) {
+                if prompt_yes!("overwrite {}?", path.quote()) {
                     Ok(())
                 } else {
                     Err(Error::Skipped)

src/uu/ln/src/ln.rs

@@ -11,7 +11,7 @@ use clap::{crate_version, Arg, ArgAction, Command};
 use uucore::display::Quotable;
 use uucore::error::{FromIo, UError, UResult};
 use uucore::fs::{make_path_relative_to, paths_refer_to_same_file};
-use uucore::{format_usage, show_error};
+use uucore::{format_usage, prompt_yes, show_error};
 
 use std::borrow::Cow;
 use std::error::Error;
@@ -19,7 +19,6 @@ use std::ffi::OsString;
 use std::fmt::Display;
 use std::fs;
 
-use std::io::stdin;
 #[cfg(any(unix, target_os = "redox"))]
 use std::os::unix::fs::symlink;
 #[cfg(windows)]
@@ -410,8 +409,7 @@ fn link(src: &Path, dst: &Path, settings: &Settings) -> UResult<()> {
         match settings.overwrite {
             OverwriteMode::NoClobber => {}
             OverwriteMode::Interactive => {
-                print!("{}: overwrite {}? ", uucore::util_name(), dst.quote());
+                if !prompt_yes!("overwrite {}?", dst.quote()) {
-                if !read_yes() {
                     return Ok(());
                 }
 
@@ -459,17 +457,6 @@ fn link(src: &Path, dst: &Path, settings: &Settings) -> UResult<()> {
     Ok(())
 }
 
-fn read_yes() -> bool {
-    let mut s = String::new();
-    match stdin().read_line(&mut s) {
-        Ok(_) => match s.char_indices().next() {
-            Some((_, x)) => x == 'y' || x == 'Y',
-            _ => false,
-        },
-        _ => false,
-    }
-}
-
 fn simple_backup_path(path: &Path, suffix: &str) -> PathBuf {
     let mut p = path.as_os_str().to_str().unwrap().to_owned();
     p.push_str(suffix);

src/uu/mv/src/mv.rs

@@ -15,7 +15,7 @@ use clap::{crate_version, error::ErrorKind, Arg, ArgAction, ArgMatches, Command}
 use std::env;
 use std::ffi::OsString;
 use std::fs;
-use std::io::{self, stdin};
+use std::io;
 #[cfg(unix)]
 use std::os::unix;
 #[cfg(windows)]
@@ -24,7 +24,7 @@ use std::path::{Path, PathBuf};
 use uucore::backup_control::{self, BackupMode};
 use uucore::display::Quotable;
 use uucore::error::{FromIo, UError, UResult, USimpleError, UUsageError};
-use uucore::{format_usage, show, show_if_err};
+use uucore::{format_usage, prompt_yes, show, show_if_err};
 
 use fs_extra::dir::{move_dir, CopyOptions as DirCopyOptions};
 
@@ -282,8 +282,7 @@ fn exec(files: &[OsString], b: &Behavior) -> UResult<()> {
                 match b.overwrite {
                     OverwriteMode::NoClobber => return Ok(()),
                     OverwriteMode::Interactive => {
-                        println!("{}: overwrite {}? ", uucore::util_name(), target.quote());
+                        if !prompt_yes!("overwrite {}? ", target.quote()) {
-                        if !read_yes() {
                             return Ok(());
                         }
                     }
@@ -377,8 +376,7 @@ fn rename(from: &Path, to: &Path, b: &Behavior) -> io::Result<()> {
         match b.overwrite {
             OverwriteMode::NoClobber => return Ok(()),
             OverwriteMode::Interactive => {
-                println!("{}: overwrite {}? ", uucore::util_name(), to.quote());
+                if !prompt_yes!("overwrite {}?", to.quote()) {
-                if !read_yes() {
                     return Ok(());
                 }
             }
@@ -494,17 +492,6 @@ fn rename_symlink_fallback(from: &Path, to: &Path) -> io::Result<()> {
     Ok(())
 }
 
-fn read_yes() -> bool {
-    let mut s = String::new();
-    match stdin().read_line(&mut s) {
-        Ok(_) => match s.chars().next() {
-            Some(x) => x == 'y' || x == 'Y',
-            _ => false,
-        },
-        _ => false,
-    }
-}
-
 fn is_empty_dir(path: &Path) -> bool {
     match fs::read_dir(path) {
         Ok(contents) => contents.peekable().peek().is_none(),

src/uu/rm/src/rm.rs

@@ -11,12 +11,12 @@ use clap::{crate_version, parser::ValueSource, Arg, ArgAction, Command};
 use remove_dir_all::remove_dir_all;
 use std::collections::VecDeque;
 use std::fs::{self, File, Metadata};
-use std::io::{stderr, stdin, BufRead, ErrorKind, Write};
+use std::io::ErrorKind;
 use std::ops::BitOr;
 use std::path::{Path, PathBuf};
 use uucore::display::Quotable;
 use uucore::error::{UResult, USimpleError, UUsageError};
-use uucore::{format_usage, show_error};
+use uucore::{format_usage, prompt_yes, show_error};
 use walkdir::{DirEntry, WalkDir};
 
 #[derive(Eq, PartialEq, Clone, Copy)]
@@ -133,11 +133,11 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
         };
         if options.interactive == InteractiveMode::Once && (options.recursive || files.len() > 3) {
             let msg = if options.recursive {
-                "Remove all arguments recursively? "
+                "Remove all arguments recursively?"
             } else {
-                "Remove all arguments? "
+                "Remove all arguments?"
             };
-            if !prompt(msg) {
+            if !prompt_yes!("{}", msg) {
                 return Ok(());
             }
         }
@@ -451,7 +451,7 @@ fn prompt_file(path: &Path, options: &Options, is_dir: bool) -> bool {
     if options.interactive == InteractiveMode::Always {
         if let Ok(metadata) = fs::symlink_metadata(path) {
             if metadata.is_symlink() {
-                return prompt(&(format!("remove symbolic link {}? ", path.quote())));
+                return prompt_yes!("remove symbolic link {}?", path.quote());
             }
         }
     }
@@ -470,25 +470,18 @@ fn prompt_file(path: &Path, options: &Options, is_dir: bool) -> bool {
                 if let Ok(metadata) = file.metadata() {
                     if metadata.permissions().readonly() {
                         if metadata.len() == 0 {
-                            prompt(
+                            prompt_yes!(
-                                &(format!(
+                                "remove write-protected regular empty file {}?",
-                                    "remove write-protected regular empty file {}? ",
                                 path.quote()
-                                )),
                             )
                         } else {
-                            prompt(
+                            prompt_yes!("remove write-protected regular file {}?", path.quote())
-                                &(format!(
-                                    "remove write-protected regular file {}? ",
-                                    path.quote()
-                                )),
-                            )
                         }
                     } else if options.interactive == InteractiveMode::Always {
                         if metadata.len() == 0 {
-                            prompt(&(format!("remove regular empty file {}? ", path.quote())))
+                            prompt_yes!("remove regular empty file {}?", path.quote())
                         } else {
-                            prompt(&(format!("remove file {}? ", path.quote())))
+                            prompt_yes!("remove file {}?", path.quote())
                         }
                     } else {
                         true
@@ -501,22 +494,15 @@ fn prompt_file(path: &Path, options: &Options, is_dir: bool) -> bool {
                 if err.kind() == ErrorKind::PermissionDenied {
                     if let Ok(metadata) = fs::metadata(path) {
                         if metadata.len() == 0 {
-                            prompt(
+                            prompt_yes!(
-                                &(format!(
+                                "remove write-protected regular empty file {}?",
-                                    "remove write-protected regular empty file {}? ",
                                 path.quote()
-                                )),
                             )
                         } else {
-                            prompt(
+                            prompt_yes!("remove write-protected regular file {}?", path.quote())
-                                &(format!(
-                                    "remove write-protected regular file {}? ",
-                                    path.quote()
-                                )),
-                            )
                         }
                     } else {
-                        prompt(&(format!("remove write-protected regular file {}? ", path.quote())))
+                        prompt_yes!("remove write-protected regular file {}?", path.quote())
                     }
                 } else {
                     true
@@ -536,9 +522,9 @@ fn handle_writable_directory(path: &Path, options: &Options, metadata: &Metadata
     // Why is S_IWUSR showing up as a u16 on macos?
     let user_writable = (mode & (libc::S_IWUSR as u32)) != 0;
     if !user_writable {
-        prompt(&(format!("remove write-protected directory {}? ", path.quote())))
+        prompt_yes!("remove write-protected directory {}?", path.quote())
     } else if options.interactive == InteractiveMode::Always {
-        prompt(&(format!("remove directory {}? ", path.quote())))
+        prompt_yes!("remove directory {}?", path.quote())
     } else {
         true
     }
@@ -551,9 +537,9 @@ fn handle_writable_directory(path: &Path, options: &Options, metadata: &Metadata
     use windows_sys::Win32::Storage::FileSystem::FILE_ATTRIBUTE_READONLY;
     let not_user_writable = (metadata.file_attributes() & FILE_ATTRIBUTE_READONLY) != 0;
     if not_user_writable {
-        prompt(&(format!("remove write-protected directory {}? ", path.quote())))
+        prompt_yes!("remove write-protected directory {}?", path.quote())
     } else if options.interactive == InteractiveMode::Always {
-        prompt(&(format!("remove directory {}? ", path.quote())))
+        prompt_yes!("remove directory {}?", path.quote())
     } else {
         true
     }
@@ -564,14 +550,14 @@ fn handle_writable_directory(path: &Path, options: &Options, metadata: &Metadata
 #[cfg(not(unix))]
 fn handle_writable_directory(path: &Path, options: &Options, metadata: &Metadata) -> bool {
     if options.interactive == InteractiveMode::Always {
-        prompt(&(format!("remove directory {}? ", path.quote())))
+        prompt_yes!("remove directory {}?", path.quote())
     } else {
         true
     }
 }
 
 fn prompt_descend(path: &Path) -> bool {
-    prompt(&(format!("descend into directory {}? ", path.quote())))
+    prompt_yes!("descend into directory {}?", path.quote())
 }
 
 fn normalize(path: &Path) -> PathBuf {
@@ -582,20 +568,6 @@ fn normalize(path: &Path) -> PathBuf {
     uucore::fs::normalize_path(path)
 }
 
-fn prompt(msg: &str) -> bool {
-    let _ = stderr().write_all(format!("{}: {}", uucore::util_name(), msg).as_bytes());
-    let _ = stderr().flush();
-
-    let mut buf = Vec::new();
-    let stdin = stdin();
-    let mut stdin = stdin.lock();
-    let read = stdin.read_until(b'\n', &mut buf);
-    match read {
-        Ok(x) if x > 0 => matches!(buf[0], b'y' | b'Y'),
-        _ => false,
-    }
-}
-
 #[cfg(not(windows))]
 fn is_symlink_dir(_metadata: &Metadata) -> bool {
     false

src/uucore/src/lib/lib.rs

@@ -166,6 +166,44 @@ pub fn args_os() -> impl Iterator<Item = OsString> {
     ARGV.iter().cloned()
 }
 
+/// Read a line from stdin and check whether the first character is `'y'` or `'Y'`
+pub fn read_yes() -> bool {
+    let mut s = String::new();
+    match std::io::stdin().read_line(&mut s) {
+        Ok(_) => matches!(s.chars().next(), Some('y' | 'Y')),
+        _ => false,
+    }
+}
+
+/// Prompt the user with a formatted string and returns `true` if they reply `'y'` or `'Y'`
+///
+/// This macro functions accepts the same syntax as `format!`. The prompt is written to
+/// `stderr`. A space is also printed at the end for nice spacing between the prompt and
+/// the user input. Any input starting with `'y'` or `'Y'` is interpreted as `yes`.
+///
+/// # Examples
+/// ```
+/// use uucore::prompt_yes;
+/// let file = "foo.rs";
+/// prompt_yes!("Do you want to delete '{}'?", file);
+/// ```
+/// will print something like below to `stderr` (with `util_name` substituted by the actual
+/// util name) and will wait for user input.
+/// ```txt
+/// util_name: Do you want to delete 'foo.rs'?
+/// ```
+#[macro_export]
+macro_rules! prompt_yes(
+    ($($args:tt)+) => ({
+        use std::io::Write;
+        eprint!("{}: ", uucore::util_name());
+        eprint!($($args)+);
+        eprint!(" ");
+        uucore::crash_if_err!(1, std::io::stderr().flush());
+        uucore::read_yes()
+    })
+);
+
 #[cfg(test)]
 mod tests {
     use super::*;

tests/by-util/test_cp.rs

@@ -236,7 +236,7 @@ fn test_cp_arg_interactive() {
         .pipe_in("N\n")
         .succeeds()
         .no_stdout()
-        .stderr_is("cp: overwrite 'b'?  [y/N]:");
+        .stderr_is("cp: overwrite 'b'? ");
 }
 
 #[test]

tests/by-util/test_ln.rs

@@ -117,7 +117,7 @@ fn test_symlink_interactive() {
         .args(&["-i", "-s", file, link])
         .pipe_in("n")
         .succeeds()
-        .no_stderr();
+        .no_stdout();
 
     assert!(at.file_exists(file));
     assert!(!at.is_symlink(link));
@@ -127,7 +127,7 @@ fn test_symlink_interactive() {
         .args(&["-i", "-s", file, link])
         .pipe_in("Yesh") // spell-checker:disable-line
         .succeeds()
-        .no_stderr();
+        .no_stdout();
 
     assert!(at.file_exists(file));
     assert!(at.is_symlink(link));

tests/by-util/test_mv.rs

@@ -166,7 +166,7 @@ fn test_mv_interactive() {
         .arg(file_b)
         .pipe_in("n")
         .succeeds()
-        .no_stderr();
+        .no_stdout();
 
     assert!(at.file_exists(file_a));
     assert!(at.file_exists(file_b));
@@ -178,7 +178,7 @@ fn test_mv_interactive() {
         .arg(file_b)
         .pipe_in("Yesh") // spell-checker:disable-line
         .succeeds()
-        .no_stderr();
+        .no_stdout();
 
     assert!(!at.file_exists(file_a));
     assert!(at.file_exists(file_b));

tests/by-util/test_rm.rs

@@ -460,24 +460,10 @@ fn test_rm_prompts() {
     let mut child: Child = scene.ucmd().arg("-ri").arg("a").run_no_wait();
 
     let mut child_stdin = child.stdin.take().unwrap();
+    for _ in 0..9 {
         child_stdin.write_all(yes.as_bytes()).unwrap();
         child_stdin.flush().unwrap();
-    child_stdin.write_all(yes.as_bytes()).unwrap();
+    }
-    child_stdin.flush().unwrap();
-    child_stdin.write_all(yes.as_bytes()).unwrap();
-    child_stdin.flush().unwrap();
-    child_stdin.write_all(yes.as_bytes()).unwrap();
-    child_stdin.flush().unwrap();
-    child_stdin.write_all(yes.as_bytes()).unwrap();
-    child_stdin.flush().unwrap();
-    child_stdin.write_all(yes.as_bytes()).unwrap();
-    child_stdin.flush().unwrap();
-    child_stdin.write_all(yes.as_bytes()).unwrap();
-    child_stdin.flush().unwrap();
-    child_stdin.write_all(yes.as_bytes()).unwrap();
-    child_stdin.flush().unwrap();
-    child_stdin.write_all(yes.as_bytes()).unwrap();
-    child_stdin.flush().unwrap();
 
     let output = child.wait_with_output().unwrap();
 
@@ -494,10 +480,10 @@ fn test_rm_prompts() {
 
     trimmed_output.sort();
 
-    assert!(trimmed_output.len() == answers.len());
+    assert_eq!(trimmed_output.len(), answers.len());
 
     for (i, checking_string) in trimmed_output.iter().enumerate() {
-        assert!(checking_string == answers[i]);
+        assert_eq!(checking_string, answers[i]);
     }
 
     assert!(!at.dir_exists("a"));
@@ -530,7 +516,10 @@ fn test_rm_force_prompts_order() {
     let output = child.wait_with_output().unwrap();
     let string_output =
         String::from_utf8(output.stderr).expect("Couldn't convert output.stderr to string");
-    assert!(string_output.trim() == "rm: remove regular empty file 'empty'?");
+    assert_eq!(
+        string_output.trim(),
+        "rm: remove regular empty file 'empty'?"
+    );
     assert!(!at.file_exists(empty_file));
 
     at.touch(empty_file);