From dfc4e3efe5558cfb89fce34cb72335a4c9dad6b1 Mon Sep 17 00:00:00 2001 From: Daniel Hofstetter Date: Mon, 2 Jun 2025 16:12:03 +0200 Subject: [PATCH 1/3] rm: do "early return" earlier in uumain --- src/uu/rm/src/rm.rs | 119 ++++++++++++++++++++++---------------------- 1 file changed, 60 insertions(+), 59 deletions(-) diff --git a/src/uu/rm/src/rm.rs b/src/uu/rm/src/rm.rs index 863336f5d..a72857131 100644 --- a/src/uu/rm/src/rm.rs +++ b/src/uu/rm/src/rm.rs @@ -119,6 +119,12 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { let force_flag = matches.get_flag(OPT_FORCE); + if files.is_empty() && !force_flag { + // Still check by hand and not use clap + // Because "rm -f" is a thing + return Err(UUsageError::new(1, "missing operand")); + } + // If -f(--force) is before any -i (or variants) we want prompts else no prompts let force_prompt_never: bool = force_flag && { let force_index = matches.index_of(OPT_FORCE).unwrap_or(0); @@ -130,71 +136,66 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { }) }; - if files.is_empty() && !force_flag { - // Still check by hand and not use clap - // Because "rm -f" is a thing - return Err(UUsageError::new(1, "missing operand")); - } else { - let options = Options { - force: force_flag, - interactive: { - if force_prompt_never { - InteractiveMode::Never - } else if matches.get_flag(OPT_PROMPT) { - InteractiveMode::Always - } else if matches.get_flag(OPT_PROMPT_MORE) { - InteractiveMode::Once - } else if matches.contains_id(OPT_INTERACTIVE) { - match matches.get_one::(OPT_INTERACTIVE).unwrap().as_str() { - "never" => InteractiveMode::Never, - "once" => InteractiveMode::Once, - "always" => InteractiveMode::Always, - val => { - return Err(USimpleError::new( - 1, - format!("Invalid argument to interactive ({val})"), - )); - } + let options = Options { + force: force_flag, + interactive: { + if force_prompt_never { + InteractiveMode::Never + } else if matches.get_flag(OPT_PROMPT) { + InteractiveMode::Always + } else if matches.get_flag(OPT_PROMPT_MORE) { + InteractiveMode::Once + } else if matches.contains_id(OPT_INTERACTIVE) { + match matches.get_one::(OPT_INTERACTIVE).unwrap().as_str() { + "never" => InteractiveMode::Never, + "once" => InteractiveMode::Once, + "always" => InteractiveMode::Always, + val => { + return Err(USimpleError::new( + 1, + format!("Invalid argument to interactive ({val})"), + )); } - } else { - InteractiveMode::PromptProtected } - }, - one_fs: matches.get_flag(OPT_ONE_FILE_SYSTEM), - preserve_root: !matches.get_flag(OPT_NO_PRESERVE_ROOT), - recursive: matches.get_flag(OPT_RECURSIVE), - dir: matches.get_flag(OPT_DIR), - verbose: matches.get_flag(OPT_VERBOSE), - __presume_input_tty: if matches.get_flag(PRESUME_INPUT_TTY) { - Some(true) } else { - None - }, - }; - if options.interactive == InteractiveMode::Once && (options.recursive || files.len() > 3) { - let msg: String = format!( - "remove {} {}{}", - files.len(), - if files.len() > 1 { - "arguments" - } else { - "argument" - }, - if options.recursive { - " recursively?" - } else { - "?" - } - ); - if !prompt_yes!("{msg}") { - return Ok(()); + InteractiveMode::PromptProtected } - } - - if remove(&files, &options) { - return Err(1.into()); + }, + one_fs: matches.get_flag(OPT_ONE_FILE_SYSTEM), + preserve_root: !matches.get_flag(OPT_NO_PRESERVE_ROOT), + recursive: matches.get_flag(OPT_RECURSIVE), + dir: matches.get_flag(OPT_DIR), + verbose: matches.get_flag(OPT_VERBOSE), + __presume_input_tty: if matches.get_flag(PRESUME_INPUT_TTY) { + Some(true) + } else { + None + }, + }; + if options.interactive == InteractiveMode::Once && (options.recursive || files.len() > 3) { + let msg: String = format!( + "remove {} {}{}", + files.len(), + if files.len() > 1 { + "arguments" + } else { + "argument" + }, + if options.recursive { + " recursively?" + } else { + "?" + } + ); + if !prompt_yes!("{msg}") { + return Ok(()); } } + + if remove(&files, &options) { + return Err(1.into()); + } + Ok(()) } From 25c7ed5b1e8213312148a951673804cbe579ad6d Mon Sep 17 00:00:00 2001 From: Daniel Hofstetter Date: Mon, 2 Jun 2025 16:19:33 +0200 Subject: [PATCH 2/3] rm: set "after help" in uu_app instead of uumain --- src/uu/rm/src/rm.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/uu/rm/src/rm.rs b/src/uu/rm/src/rm.rs index a72857131..3364007b1 100644 --- a/src/uu/rm/src/rm.rs +++ b/src/uu/rm/src/rm.rs @@ -110,7 +110,7 @@ static ARG_FILES: &str = "files"; #[uucore::main] pub fn uumain(args: impl uucore::Args) -> UResult<()> { - let matches = uu_app().after_help(AFTER_HELP).try_get_matches_from(args)?; + let matches = uu_app().try_get_matches_from(args)?; let files: Vec<&OsStr> = matches .get_many::(ARG_FILES) @@ -204,6 +204,7 @@ pub fn uu_app() -> Command { .version(uucore::crate_version!()) .about(ABOUT) .override_usage(format_usage(USAGE)) + .after_help(AFTER_HELP) .infer_long_args(true) .args_override_self(true) .arg( From 3eede813db9259e57cfee6cb254f312d73aa04c0 Mon Sep 17 00:00:00 2001 From: Daniel Hofstetter Date: Mon, 2 Jun 2025 16:22:40 +0200 Subject: [PATCH 3/3] rm: remove some unnecessary type information --- src/uu/rm/src/rm.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/uu/rm/src/rm.rs b/src/uu/rm/src/rm.rs index 3364007b1..3f48e311b 100644 --- a/src/uu/rm/src/rm.rs +++ b/src/uu/rm/src/rm.rs @@ -112,7 +112,7 @@ static ARG_FILES: &str = "files"; pub fn uumain(args: impl uucore::Args) -> UResult<()> { let matches = uu_app().try_get_matches_from(args)?; - let files: Vec<&OsStr> = matches + let files: Vec<_> = matches .get_many::(ARG_FILES) .map(|v| v.map(OsString::as_os_str).collect()) .unwrap_or_default(); @@ -126,7 +126,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { } // If -f(--force) is before any -i (or variants) we want prompts else no prompts - let force_prompt_never: bool = force_flag && { + let force_prompt_never = force_flag && { let force_index = matches.index_of(OPT_FORCE).unwrap_or(0); ![OPT_PROMPT, OPT_PROMPT_MORE, OPT_INTERACTIVE] .iter()