From 6cdcfca573357825e762a5742224b627f22255cd Mon Sep 17 00:00:00 2001 From: Terts Diepraam Date: Wed, 16 Nov 2022 15:11:44 +0100 Subject: [PATCH 1/5] uucore: add prompt_yes!() and read_yes() These functions are based on existing functions and macros in utils `cp`, `ln`, `mv ` and `rm`. This unifies the separate implementations. --- src/uucore/src/lib/lib.rs | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/src/uucore/src/lib/lib.rs b/src/uucore/src/lib/lib.rs index 662a371c4..66f8881d6 100644 --- a/src/uucore/src/lib/lib.rs +++ b/src/uucore/src/lib/lib.rs @@ -166,6 +166,44 @@ pub fn args_os() -> impl Iterator { 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::*; From ed34264b95bb175012c8144b2659da4908e130d9 Mon Sep 17 00:00:00 2001 From: Terts Diepraam Date: Wed, 16 Nov 2022 15:15:02 +0100 Subject: [PATCH 2/5] cp: use uucore prompt_yes instead of custom --- src/uu/cp/src/cp.rs | 24 +++--------------------- tests/by-util/test_cp.rs | 2 +- 2 files changed, 4 insertions(+), 22 deletions(-) diff --git a/src/uu/cp/src/cp.rs b/src/uu/cp/src/cp.rs index 9618e39cb..0dddceab6 100644 --- a/src/uu/cp/src/cp.rs +++ b/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 = Result; 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) diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs index b1ef80fcc..c3afba730 100644 --- a/tests/by-util/test_cp.rs +++ b/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] From 7bb0e8f8492fedafcf378c25a22ddbd95100fd8b Mon Sep 17 00:00:00 2001 From: Terts Diepraam Date: Wed, 16 Nov 2022 15:15:42 +0100 Subject: [PATCH 3/5] ln: use uucore::prompt_yes over custom function --- src/uu/ln/src/ln.rs | 17 ++--------------- tests/by-util/test_ln.rs | 4 ++-- 2 files changed, 4 insertions(+), 17 deletions(-) diff --git a/src/uu/ln/src/ln.rs b/src/uu/ln/src/ln.rs index 65b5f88dd..d9d20686e 100644 --- a/src/uu/ln/src/ln.rs +++ b/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 !read_yes() { + if !prompt_yes!("overwrite {}?", dst.quote()) { 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); diff --git a/tests/by-util/test_ln.rs b/tests/by-util/test_ln.rs index cb4729adf..07b824649 100644 --- a/tests/by-util/test_ln.rs +++ b/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)); From 91df2b1709d356bf1154cab99b93e6398b61245a Mon Sep 17 00:00:00 2001 From: Terts Diepraam Date: Wed, 16 Nov 2022 15:16:03 +0100 Subject: [PATCH 4/5] mv: use uucore::prompt_yes over custom function --- src/uu/mv/src/mv.rs | 21 ++++----------------- tests/by-util/test_mv.rs | 4 ++-- 2 files changed, 6 insertions(+), 19 deletions(-) diff --git a/src/uu/mv/src/mv.rs b/src/uu/mv/src/mv.rs index f5b6f4cd7..bcefc39a6 100644 --- a/src/uu/mv/src/mv.rs +++ b/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 !read_yes() { + if !prompt_yes!("overwrite {}? ", target.quote()) { 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 !read_yes() { + if !prompt_yes!("overwrite {}?", to.quote()) { 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(), diff --git a/tests/by-util/test_mv.rs b/tests/by-util/test_mv.rs index 55ac7d68d..b4470ccea 100644 --- a/tests/by-util/test_mv.rs +++ b/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)); From 33cbc94f25b9c4eaa3c589f99c7b1d217febf457 Mon Sep 17 00:00:00 2001 From: Terts Diepraam Date: Wed, 16 Nov 2022 15:16:39 +0100 Subject: [PATCH 5/5] rm: use uucore::prompt_yes over custom prompt function --- src/uu/rm/src/rm.rs | 74 +++++++++++++--------------------------- tests/by-util/test_rm.rs | 31 ++++++----------- 2 files changed, 33 insertions(+), 72 deletions(-) diff --git a/src/uu/rm/src/rm.rs b/src/uu/rm/src/rm.rs index 78ba25306..56da7155c 100644 --- a/src/uu/rm/src/rm.rs +++ b/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( - &(format!( - "remove write-protected regular empty file {}? ", - path.quote() - )), + prompt_yes!( + "remove write-protected regular empty file {}?", + path.quote() ) } else { - prompt( - &(format!( - "remove write-protected regular file {}? ", - path.quote() - )), - ) + prompt_yes!("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( - &(format!( - "remove write-protected regular empty file {}? ", - path.quote() - )), + prompt_yes!( + "remove write-protected regular empty file {}?", + path.quote() ) } else { - prompt( - &(format!( - "remove write-protected regular file {}? ", - path.quote() - )), - ) + prompt_yes!("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 diff --git a/tests/by-util/test_rm.rs b/tests/by-util/test_rm.rs index 98273fedd..ee81cf8d9 100644 --- a/tests/by-util/test_rm.rs +++ 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(); - 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(); + for _ in 0..9 { + 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);