diff --git a/src/clap/streaming.rs b/src/clap/streaming.rs index d7f59518e819..c0b2e836a845 100644 --- a/src/clap/streaming.rs +++ b/src/clap/streaming.rs @@ -242,16 +242,17 @@ where } else { false }; - if param.takes_value == clap::Values::None - || param.takes_value == clap::Values::OneOptional - { - if next_is_eql && param.takes_value == clap::Values::None { + if param.takes_value == clap::Values::None { + if next_is_eql { return Err(self.err(arg, Some(short), None, ArgError::DoesntTakeValue)); } return Ok(Some(Arg { param, value: None })); } if arg.len() <= next_index { + if param.takes_value == clap::Values::OneOptional { + return Ok(Some(Arg { param, value: None })); + } let value = match self.iter.next() { Some(v) => v, None => { @@ -554,6 +555,90 @@ mod tests { ); } + #[test] + fn short_one_optional() { + let params: [clap::Param; 3] = [ + clap::Param { + id: 0, + names: clap::Names { + short: Some(b'a'), + ..Default::default() + }, + ..Default::default() + }, + clap::Param { + id: 1, + names: clap::Names { + short: Some(b'c'), + ..Default::default() + }, + takes_value: clap::Values::OneOptional, + ..Default::default() + }, + clap::Param { + id: 2, + takes_value: clap::Values::One, + ..Default::default() + }, + ]; + let a = ¶ms[0]; + let c = ¶ms[1]; + let pos = ¶ms[2]; + + test_no_err( + ¶ms, + &[ + b"-c", b"-c=v", b"-cv", b"-ac", b"-ac=v", b"-acv", b"-c", b"p", + ], + &[ + Arg { + param: c, + value: None, + }, + Arg { + param: c, + value: Some(b"v"), + }, + Arg { + param: c, + value: Some(b"v"), + }, + Arg { + param: a, + value: None, + }, + Arg { + param: c, + value: None, + }, + Arg { + param: a, + value: None, + }, + Arg { + param: c, + value: Some(b"v"), + }, + Arg { + param: a, + value: None, + }, + Arg { + param: c, + value: Some(b"v"), + }, + Arg { + param: c, + value: None, + }, + Arg { + param: pos, + value: Some(b"p"), + }, + ], + ); + } + #[test] fn long_params() { let params: [clap::Param; 4] = [ diff --git a/src/install/PackageManager/CommandLineArguments.rs b/src/install/PackageManager/CommandLineArguments.rs index be895af25bf1..1d5711427668 100644 --- a/src/install/PackageManager/CommandLineArguments.rs +++ b/src/install/PackageManager/CommandLineArguments.rs @@ -51,7 +51,7 @@ const BACKEND_PARAM: ParamType = clap::param!( ); const SHARED_HEAD_PARAMS: &[ParamType] = &[ - clap::param!("-c, --config ? Specify path to config file (bunfig.toml)"), + clap::param!("-c, --config Specify path to config file (bunfig.toml)"), clap::param!("-y, --yarn Write a yarn.lock file (yarn v1)"), ]; diff --git a/src/runtime/cli/Arguments.rs b/src/runtime/cli/Arguments.rs index ab0fc39e9b37..e613e6fc2d4e 100644 --- a/src/runtime/cli/Arguments.rs +++ b/src/runtime/cli/Arguments.rs @@ -100,7 +100,7 @@ const BASE_PARAMS_: &[ParamType] = concat_params!( "--cwd Absolute path to resolve files & entry points from. This just changes the process' cwd." ), parse_param!( - "-c, --config ? Specify path to Bun config file. Default $cwd/bunfig.toml" + "-c, --config Specify path to Bun config file. Default $cwd/bunfig.toml" ), parse_param!("-h, --help Display this menu and exit"), ], diff --git a/test/cli/install/bun-run.test.ts b/test/cli/install/bun-run.test.ts index 2dac7325e97c..3db076339da5 100644 --- a/test/cli/install/bun-run.test.ts +++ b/test/cli/install/bun-run.test.ts @@ -221,11 +221,10 @@ describe.concurrent("bun run", () => { 2, ), "index.js": "console.log('hi')", - ...(withLogLevel ? { "bunfig.toml": `logLevel = "debug"` } : {}), + "bunfig.toml": withLogLevel ? `logLevel = "debug"` : ``, }); await using proc = Bun.spawn({ - // TODO: figure out why -c is necessary here. cmd: [ bunExe(), ...(withRun ? ["run"] : []), diff --git a/test/config/bunfig/config-flag.test.ts b/test/config/bunfig/config-flag.test.ts new file mode 100644 index 000000000000..d52ed3807370 --- /dev/null +++ b/test/config/bunfig/config-flag.test.ts @@ -0,0 +1,99 @@ +import { describe, expect, test } from "bun:test"; +import { bunEnv, bunExe, tempDir } from "harness"; +import { join } from "path"; + +// `-c` / `--config` must bind its path argument in every spelling. +// https://github.com/oven-sh/bun/issues/6300, https://github.com/oven-sh/bun/issues/21431 + +async function run(cwd: string, argv: string[]) { + await using proc = Bun.spawn({ + cmd: [bunExe(), ...argv], + env: bunEnv, + cwd, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + return { stdout, stderr, exitCode }; +} + +const spellings = [ + ["--config=cfg.toml"], + ["--config", "cfg.toml"], + ["-c=cfg.toml"], + ["-c", "cfg.toml"], + ["-ccfg.toml"], +]; + +describe.concurrent("bun install --config path binding", () => { + test.each(spellings)("install %p loads the named config, never treats it as a package", async (...flag) => { + // Local 404 registry so nothing ever reaches the public network, even if + // parsing regresses and routes the path to `bun add`. + const hits: string[] = []; + await using server = Bun.serve({ + port: 0, + fetch(req) { + hits.push(new URL(req.url).pathname); + return new Response("{}", { status: 404 }); + }, + }); + const base = `http://localhost:${server.port}`; + + using dir = tempDir("config-flag-install", { + "package.json": JSON.stringify({ name: "x", version: "1.0.0", dependencies: { "no-deps": "1.0.0" } }), + // Auto-loaded when `--config` is NOT effective (regression). + "bunfig.toml": `[install]\nregistry = "${base}/default/"\ncache = false\n`, + // Loaded when `--config cfg.toml` IS effective (the fix). + "cfg.toml": `[install]\nregistry = "${base}/fromcfg/"\ncache = false\n`, + }); + + const { stdout, stderr, exitCode } = await run(String(dir), ["install", ...flag]); + const out = stdout + stderr; + + // The path must never be routed to `bun add` as a package name. + expect(out).not.toContain("bun add"); + expect(hits).not.toContain("/default/cfg.toml"); + // The named config must have been loaded, not the auto-loaded bunfig.toml. + expect(hits).toContain("/fromcfg/no-deps"); + expect(hits).not.toContain("/default/no-deps"); + expect(exitCode).not.toBe(0); + }); +}); + +describe.concurrent("bare -c / --config requires a value", () => { + test.each([[["install", "--config"]], [["install", "-c"]], [["--config"]], [["-c"]]])( + "bun %p errors instead of silently defaulting", + async argv => { + using dir = tempDir("config-flag-bare", { + "package.json": JSON.stringify({ name: "x", version: "1.0.0" }), + }); + const { stderr, exitCode } = await run(String(dir), argv); + expect(stderr).toContain(argv.at(-1)!); + expect(stderr.toLowerCase()).toContain("requires a value"); + expect(exitCode).not.toBe(0); + }, + ); +}); + +// The subcommand keyword is picked before flag values are parsed, so only the +// single-token spellings can precede `run`; the space forms go after it. +const scriptArgv = [ + ...spellings, + ...spellings.map(flag => ["run", ...flag]), + ...spellings.filter(flag => flag.length === 1).map(flag => [...flag, "run"]), +]; + +describe.concurrent("bun [run] --config path binding", () => { + test.each(scriptArgv.map(argv => [argv]))("bun %p app.ts loads the named config and runs app.ts", async argv => { + using dir = tempDir("config-flag-run", { + "app.ts": `console.log(JSON.stringify({ fromCfg: process.env.FROM_CFG ?? "no", argv: process.argv.slice(2) }));`, + "cfg.toml": `[define]\n"process.env.FROM_CFG" = '"yes"'\n`, + }); + + const { stdout, stderr, exitCode } = await run(String(dir), [...argv, join(String(dir), "app.ts"), "pass-through"]); + // The config path must not be treated as the entry point. + expect(stderr).not.toContain("cfg.toml"); + expect(JSON.parse(stdout.trim())).toEqual({ fromCfg: "yes", argv: ["pass-through"] }); + expect(exitCode).toBe(0); + }); +});