Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions src/runtime/shell/builtin/basename.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,12 @@ impl Basename {
let buf = {
let bltn = Builtin::of(interp, cmd);
let argc = bltn.args_slice().len();
if argc == 0 {
let start = (argc != 0 && bltn.arg_bytes(0) == b"--") as usize;
if start >= argc {
return Self::fail(interp, cmd, Kind::Basename.usage_string());
}
let mut buf = Vec::new();
for i in 0..argc {
for i in start..argc {
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
buf.extend_from_slice(bun_paths::resolve_path::basename(bltn.arg_bytes(i)));
buf.push(b'\n');
}
Expand Down
8 changes: 5 additions & 3 deletions src/runtime/shell/builtin/cd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,16 +23,18 @@ enum State {
impl Cd {
pub(crate) fn start(interp: &Interpreter, cmd: NodeId) -> Yield {
let args = Builtin::of(interp, cmd).args_slice();
if args.len() > 1 {
let skip =
(!args.is_empty() && Builtin::of(interp, cmd).arg_bytes(0) == b"--") as usize;
if args.len() - skip > 1 {
return Self::write_stderr_non_blocking(
interp,
cmd,
format_args!("too many arguments\n"),
);
}

if args.len() == 1 {
let first_arg = Builtin::of(interp, cmd).arg_bytes(0);
if args.len() - skip == 1 {
let first_arg = Builtin::of(interp, cmd).arg_bytes(skip);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
if first_arg == b"-" {
let prev = Builtin::shell(interp, cmd).prev_cwd().to_vec();
if let Err(err) = interp.as_cmd_mut(cmd).base.shell_mut().change_prev_cwd() {
Expand Down
5 changes: 3 additions & 2 deletions src/runtime/shell/builtin/dirname.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,14 @@ impl Dirname {
pub(crate) fn start(interp: &Interpreter, cmd: NodeId) -> Yield {
let bltn = Builtin::of(interp, cmd);
let argc = bltn.args_slice().len();
if argc == 0 {
let start = (argc != 0 && bltn.arg_bytes(0) == b"--") as usize;
if start >= argc {
return Self::fail(interp, cmd, b"usage: dirname string\n");
}

let stdout_needs_io = bltn.stdout.needs_io();
let mut buf = Vec::new();
for i in 0..argc {
for i in start..argc {
let path = bltn.arg_bytes(i);
let dir = bun_paths::resolve_path::dirname::<bun_paths::platform::Posix>(path);
let dir: &[u8] = if dir.is_empty() { b"." } else { dir };
Expand Down
4 changes: 4 additions & 0 deletions src/runtime/shell/builtin/ls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,10 @@ impl Ls {
let mut idx = 0usize;
while idx < argc {
let flag = Builtin::of(interp, cmd).arg_bytes(idx);
if flag == b"--" {
idx += 1;
return Ok(if idx < argc { Some(idx) } else { None });
}
match Self::parse_flag(&mut Self::state_mut(interp, cmd).opts, flag) {
ParseFlag::Done => return Ok(Some(idx)),
ParseFlag::ContinueParsing => {}
Expand Down
27 changes: 14 additions & 13 deletions src/runtime/shell/builtin/mv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -350,23 +350,24 @@ impl Mv {
let mut idx = 0usize;
while idx < argc {
let flag = Builtin::of(interp, cmd).arg_bytes(idx);
if flag == b"--" {
idx += 1;
break;
}
match Self::parse_flag(&mut Self::state_mut(interp, cmd).opts, flag) {
MvFlag::Done => {
let filepath_args = argc - idx;
if filepath_args < 2 {
return Err(MvParseError::ShowUsage);
}
let me = Self::state_mut(interp, cmd);
me.args.sources_start = idx;
me.args.target_idx = argc - 1;
return Ok(());
}
MvFlag::ContinueParsing => {}
MvFlag::Done => break,
MvFlag::ContinueParsing => idx += 1,
MvFlag::IllegalOption(s) => return Err(MvParseError::IllegalOption(s)),
}
idx += 1;
}
Err(MvParseError::ShowUsage)
let filepath_args = argc - idx;
if filepath_args < 2 {
return Err(MvParseError::ShowUsage);
}
let me = Self::state_mut(interp, cmd);
me.args.sources_start = idx;
me.args.target_idx = argc - 1;
Ok(())
}

fn parse_flag(opts: &mut Opts, flag: &[u8]) -> MvFlag {
Expand Down
14 changes: 12 additions & 2 deletions src/runtime/shell/builtin/rm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,13 @@ impl Rm {
}

let arg = Builtin::of(interp, cmd).arg_bytes(idx as usize).to_vec();
match Self::parse_flag(&mut Self::state_mut(interp, cmd).opts, &arg) {
let is_end_of_options = arg == b"--";
let parsed = if is_end_of_options {
RmParseFlag::Done
} else {
Self::parse_flag(&mut Self::state_mut(interp, cmd).opts, &arg)
};
match parsed {
RmParseFlag::ContinueParsing => {
if let RmState::ParseOpts { idx: i, .. } =
&mut Self::state_mut(interp, cmd).state
Expand All @@ -174,7 +180,11 @@ impl Rm {
return Self::write_err_literal(interp, cmd, idx, buf);
}

let args_start = idx as usize;
let args_start = idx as usize + is_end_of_options as usize;
if args_start >= argc {
let usage = Kind::Rm.usage_string();
return Self::write_err_literal(interp, cmd, idx, usage);
}

// Check that none of the paths will delete the root.
{
Expand Down
3 changes: 3 additions & 0 deletions src/runtime/shell/builtin/seq.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,9 @@ impl Seq {
idx += 1;
continue;
}
if arg == b"--" {
idx += 1;
}
break;
}

Expand Down
7 changes: 4 additions & 3 deletions src/runtime/shell/builtin/which.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@ pub enum State {
impl Which {
pub(crate) fn start(interp: &Interpreter, cmd: NodeId) -> Yield {
let argc = Builtin::of(interp, cmd).args_slice().len();
if argc == 0 {
let start = (argc != 0 && Builtin::of(interp, cmd).arg_bytes(0) == b"--") as usize;
if start >= argc {
if let Some(safeguard) = Builtin::of(interp, cmd).stdout.needs_io() {
Self::state_mut(interp, cmd).state = State::OneArg;
let child = ChildPtr::new(cmd, WriterTag::Builtin);
Expand All @@ -49,7 +50,7 @@ impl Which {
// captured buffer, then finish.
let (path_env, cwd) = Self::path_and_cwd(interp, cmd);
let mut had_not_found = false;
for i in 0..argc {
for i in start..argc {
let arg = Self::arg(interp, cmd, i);
match Self::resolve(&path_env, &cwd, &arg) {
Some(resolved) => {
Expand Down Expand Up @@ -79,7 +80,7 @@ impl Which {
}

Self::state_mut(interp, cmd).state = State::MultiArgs {
arg_idx: 0,
arg_idx: start,
had_not_found: false,
waiting_write: false,
};
Expand Down
7 changes: 4 additions & 3 deletions src/runtime/shell/builtin/yes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,13 @@ impl Yes {
pub(crate) fn start(interp: &Interpreter, cmd: NodeId) -> Yield {
// Build one copy of the output line.
let argc = Builtin::of(interp, cmd).args_slice().len();
let start = (argc != 0 && Builtin::of(interp, cmd).arg_bytes(0) == b"--") as usize;
let mut one = Vec::new();
if argc == 0 {
if start >= argc {
one.extend_from_slice(b"y\n");
} else {
for i in 0..argc {
if i > 0 {
for i in start..argc {
if i > start {
one.push(b' ');
}
one.extend_from_slice(Builtin::of(interp, cmd).arg_bytes(i));
Expand Down
4 changes: 4 additions & 0 deletions src/runtime/shell/interpreter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2577,6 +2577,10 @@
while idx < args.len() {
// SAFETY: argv entries are NUL-terminated C strings (see Builtin::init).
let flag = unsafe { bun_core::ffi::cstr(args[idx]) }.to_bytes();
if flag == b"--" {
let rest = &args[idx + 1..];
return Ok(if rest.is_empty() { None } else { Some(rest) });
}

Check warning on line 2583 in src/runtime/shell/interpreter.rs

View check run for this annotation

Claude / Claude Code Review

pwd/exit/export builtins still reject -- delimiter

nit: `pwd --`, `exit -- 5`, and `export --` still fail — pwd.rs:30 rejects any arg with "too many arguments", exit.rs matches on argc so 2 args errors, and export.rs iterates from 0 so it treats `--` as a var name. Bash accepts `--` for all three per POSIX Guideline 10. Since this PR already added the `--` skip to other builtins without flag parsers (basename, dirname, cd, which, yes), it'd be nice to cover these three too — or note them as intentionally deferred. Not blocking; the PR is a stric
Comment thread
robobun marked this conversation as resolved.
match parse_one_flag(opts, flag) {
ParseFlagResult::Done => return Ok(Some(&args[idx..])),
ParseFlagResult::ContinueParsing => {}
Expand Down
192 changes: 192 additions & 0 deletions test/js/bun/shell/commands/double-dash.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,192 @@
// POSIX Utility Syntax Guideline 10: `--` ends option parsing; any following
// arguments are operands even if they begin with `-`.
import { $ } from "bun";
import { describe, expect, test } from "bun:test";
import { createTestBuilder } from "../test_builder";
import { sortedShellOutput } from "../util";
const TestBuilder = createTestBuilder(import.meta.path);

$.nothrow();

describe("-- end-of-options delimiter", () => {
describe("rm", () => {
TestBuilder.command`touch a; rm -- a`
.ensureTempDir()
.stderr("")
.exitCode(0)
.doesNotExist("a")
.runAsTest("rm -- file");

TestBuilder.command`touch a; rm -v -- a`
.ensureTempDir()
.stdout("a\n")
.stderr("")
.exitCode(0)
.doesNotExist("a")
.runAsTest("rm -v -- file applies flag before --");

TestBuilder.command`touch ./-f; rm -- -f`
.ensureTempDir()
.stderr("")
.exitCode(0)
.doesNotExist("-f")
.runAsTest("rm -- -f treats -f as an operand");

TestBuilder.command`rm --`
.ensureTempDir()
.stderr("usage: rm [-f | -i] [-dIPRrvWx] file ...\n unlink [--] file\n")
.exitCode(1)
.runAsTest("rm -- with no operands shows usage");
});

describe("mv", () => {
TestBuilder.command`echo hi > a; mv -- a b`
.ensureTempDir()
.stderr("")
.exitCode(0)
.doesNotExist("a")
.fileEquals("b", "hi\n")
.runAsTest("mv -- src dst");

TestBuilder.command`echo hi > ./-n; mv -- -n out`
.ensureTempDir()
.stderr("")
.exitCode(0)
.doesNotExist("-n")
.fileEquals("out", "hi\n")
.runAsTest("mv -- -n out treats -n as an operand");

TestBuilder.command`mv -- a`
.ensureTempDir()
.stderr("usage: mv [-f | -i | -n] [-hv] source target\n mv [-f | -i | -n] [-v] source ... directory\n")
.exitCode(1)
.runAsTest("mv -- with one operand shows usage");
});

describe("mkdir", () => {
TestBuilder.command`mkdir -- d; ls`
.ensureTempDir()
.stdout("d\n")
.stderr("")
.exitCode(0)
.runAsTest("mkdir -- dir");

TestBuilder.command`mkdir -p -- a/b; ls a`
.ensureTempDir()
.stdout("b\n")
.stderr("")
.exitCode(0)
.runAsTest("mkdir -p -- nested applies flag before --");

TestBuilder.command`mkdir -- -p; ls`
.ensureTempDir()
.stdout("-p\n")
.stderr("")
.exitCode(0)
.runAsTest("mkdir -- -p treats -p as an operand");
});

describe("touch", () => {
TestBuilder.command`touch -- t; ls`
.ensureTempDir()
.stdout("t\n")
.stderr("")
.exitCode(0)
.runAsTest("touch -- file");

TestBuilder.command`touch -- -a; ls`
.ensureTempDir()
.stdout("-a\n")
.stderr("")
.exitCode(0)
.runAsTest("touch -- -a treats -a as an operand");
});

describe("ls", () => {
TestBuilder.command`touch x; ls --`
.ensureTempDir()
.stdout("x\n")
.stderr("")
.exitCode(0)
.runAsTest("ls -- with no operands lists cwd");

TestBuilder.command`touch a b; ls -- a b`
.ensureTempDir()
.stdout(str => expect(sortedShellOutput(str)).toEqual(["a", "b"]))
.stderr("")
.exitCode(0)
.runAsTest("ls -- file file");

TestBuilder.command`touch ./-a; ls -- -a`
.ensureTempDir()
.stdout("-a\n")
.stderr("")
.exitCode(0)
.runAsTest("ls -- -a treats -a as an operand");
});

describe("seq", () => {
TestBuilder.command`seq -- 2`.stdout("1\n2\n").stderr("").exitCode(0).runAsTest("seq -- 2");

TestBuilder.command`seq -s , -- 3`.stdout("1,2,3,").stderr("").exitCode(0).runAsTest("seq -s , -- 3");

TestBuilder.command`seq --`
.stderr("usage: seq [-w] [-f format] [-s string] [-t string] [first [incr]] last\n")
.exitCode(1)
.runAsTest("seq -- with no operands shows usage");
});

describe("cd", () => {
TestBuilder.command`mkdir sub; cd -- sub && echo ok`
.ensureTempDir()
.stdout("ok\n")
.stderr("")
.exitCode(0)
.runAsTest("cd -- dir");

TestBuilder.command`cd -- && echo ok`.stdout("ok\n").stderr("").exitCode(0).runAsTest("cd -- with no operand");
});

describe("basename", () => {
TestBuilder.command`basename -- /a/b`.stdout("b\n").stderr("").exitCode(0).runAsTest("basename -- path");

TestBuilder.command`basename --`
.stderr("usage: basename string\n")
.exitCode(1)
.runAsTest("basename -- with no operand shows usage");
});

describe("dirname", () => {
TestBuilder.command`dirname -- /a/b`.stdout("/a\n").stderr("").exitCode(0).runAsTest("dirname -- path");

TestBuilder.command`dirname --`
.stderr("usage: dirname string\n")
.exitCode(1)
.runAsTest("dirname -- with no operand shows usage");
});

describe("which", () => {
TestBuilder.command`which -- bun_nope_not_a_thing`
.stdout(str => {
expect(str).not.toContain("--");
expect(str).toContain("bun_nope_not_a_thing not found\n");
})
.stderr("")
.exitCode(1)
.runAsTest("which -- name does not treat -- as an operand");
Comment thread
coderabbitai[bot] marked this conversation as resolved.
});

describe("yes", () => {
test("yes -- outputs 'y'", async () => {
const buffer = Buffer.alloc(6);
await $`yes -- > ${buffer}`;
expect(buffer.toString()).toEqual("y\ny\ny\n");
});

test("yes -- -n outputs '-n'", async () => {
const buffer = Buffer.alloc(6);
await $`yes -- -n > ${buffer}`;
expect(buffer.toString()).toEqual("-n\n-n\n");
});
});
});
Loading