diff --git a/src/runtime/shell/builtin/rm.rs b/src/runtime/shell/builtin/rm.rs index 8c4cde84e18d..f3afaa846181 100644 --- a/src/runtime/shell/builtin/rm.rs +++ b/src/runtime/shell/builtin/rm.rs @@ -7,7 +7,8 @@ use bun_sys::{E, FdExt, dir_iterator}; use crate::shell::ExitCode; use crate::shell::builtin::{Builtin, IoKind, Kind}; use crate::shell::interpreter::{ - EventLoopHandle, Interpreter, NodeId, ShellTask, WorkPoolTask, shell_openat, + EventLoopHandle, Interpreter, NodeId, ParseError, ShellTask, WorkPoolTask, shell_openat, + unsupported_flag, }; use crate::shell::io_writer::{ChildPtr, WriterTag}; use crate::shell::yield_::Yield; @@ -80,13 +81,11 @@ impl Default for Opts { #[derive(Default, Clone, Copy)] pub enum PromptBehaviour { - /// `--interactive=never` (default) + /// `-f`, `--interactive=never` (default) #[default] Never, - /// `-I`, `--interactive=once` - Once { removed_count: u32 }, - /// `-i`, `--interactive=always` - Always, + /// `-i`, `-I`, `--interactive=once|always`, spelled as given so the rejection can name it. + Prompt { flag: &'static [u8] }, } enum RmParseFlag { @@ -158,12 +157,22 @@ impl Rm { opts.remove_empty_dirs = true; } } - if !matches!( - Self::state_mut(interp, cmd).opts.prompt_behaviour, - PromptBehaviour::Never - ) { - let buf: &[u8] = b"rm: \"-i\" is not supported yet"; - return Self::write_err_literal(interp, cmd, idx, buf); + // After all flags, so a later `-f` cancels an earlier `-i`. + if let PromptBehaviour::Prompt { flag } = + Self::state_mut(interp, cmd).opts.prompt_behaviour + { + return Builtin::fail_parse( + interp, + cmd, + Kind::Rm, + &ParseError::Unsupported(unsupported_flag(flag)), + || { + Self::state_mut(interp, cmd).state = RmState::ParseOpts { + idx, + wait_write_err: true, + } + }, + ); } let args_start = idx as usize; @@ -511,11 +520,15 @@ impl Rm { RmParseFlag::ContinueParsing } b"--interactive=once" => { - opts.prompt_behaviour = PromptBehaviour::Once { removed_count: 0 }; + opts.prompt_behaviour = PromptBehaviour::Prompt { + flag: b"--interactive=once", + }; RmParseFlag::ContinueParsing } b"--interactive=always" => { - opts.prompt_behaviour = PromptBehaviour::Always; + opts.prompt_behaviour = PromptBehaviour::Prompt { + flag: b"--interactive=always", + }; RmParseFlag::ContinueParsing } _ => RmParseFlag::IllegalOption, @@ -530,8 +543,8 @@ impl Rm { b'r' | b'R' => opts.recursive = true, b'v' => opts.verbose = true, b'd' => opts.remove_empty_dirs = true, - b'i' => opts.prompt_behaviour = PromptBehaviour::Once { removed_count: 0 }, - b'I' => opts.prompt_behaviour = PromptBehaviour::Always, + b'i' => opts.prompt_behaviour = PromptBehaviour::Prompt { flag: b"-i" }, + b'I' => opts.prompt_behaviour = PromptBehaviour::Prompt { flag: b"-I" }, _ => return RmParseFlag::IllegalOptionWithFlag, } } diff --git a/test/js/bun/shell/commands/rm.test.ts b/test/js/bun/shell/commands/rm.test.ts index d442eb197773..5b1d09f9150e 100644 --- a/test/js/bun/shell/commands/rm.test.ts +++ b/test/js/bun/shell/commands/rm.test.ts @@ -145,6 +145,77 @@ foo/ } }); + // Prompting is not implemented. A prompting option still in effect after all + // flags are parsed is rejected under the name it was given and the operand is + // left alone; a later -f or --interactive=never cancels it, as in GNU rm. + describe("interactive flags", () => { + const unsupported = (flag: string) => `rm: unsupported option, please open a GitHub issue -- ${flag}\n`; + + // flags as given -> the option the message names (the last prompting option given) + const rejected: [flags: string[], reported: string][] = [ + [["-i"], "-i"], + [["-I"], "-I"], + [["--interactive=always"], "--interactive=always"], + [["--interactive=once"], "--interactive=once"], + [["-ri"], "-i"], + [["-rfI"], "-I"], + [["-fi"], "-i"], + [["-iI"], "-I"], + [["-I", "-i"], "-i"], + [["--interactive=once", "-i"], "-i"], + [["-i", "--interactive=once"], "--interactive=once"], + [["--interactive=always", "-I"], "-I"], + [["-I", "--interactive=always"], "--interactive=always"], + ]; + for (const [flags, reported] of rejected) { + const name = `rm ${flags.join(" ")} file`; + // Not quiet: stderr is a real fd and the message is written asynchronously. + TestBuilder.command`rm ${flags} file` + .file("file", "keep") + .stdout("") + .stderr(unsupported(reported)) + .exitCode(1) + .fileEquals("file", "keep") + .runAsTest(name); + // Quiet: stderr is captured into a buffer and written synchronously. + TestBuilder.command`rm ${flags} file` + .quiet() + .file("file", "keep") + .stdout("") + .stderr(unsupported(reported)) + .exitCode(1) + .fileEquals("file", "keep") + .runAsTest(`${name} (quiet)`); + } + + const cancelled: string[][] = [ + ["-if"], + ["-i", "-f"], + ["-I", "-f"], + ["-Irf"], + ["--interactive=once", "-f"], + ["--interactive=always", "-f"], + ["-i", "--interactive=never"], + ["-I", "--interactive=never"], + ]; + for (const flags of cancelled) { + TestBuilder.command`rm ${flags} file` + .file("file", "") + .stdout("") + .stderr("") + .exitCode(0) + .doesNotExist("file") + .runAsTest(`rm ${flags.join(" ")} file removes the file`); + } + + // With no operand the usage error comes first, as for any other flags. + TestBuilder.command`rm -i` + .stdout("") + .stderr("usage: rm [-f | -i] [-dIPRrvWx] file ...\n unlink [--] file\n") + .exitCode(1) + .runAsTest("rm -i without an operand prints usage"); + }); + // The DirTask parent/child hand-off had a lost-wakeup window between // `subtask_count.load() > 1` and `need_to_wait.store(true)`: the last // child could decrement and read `need_to_wait == false` in between,