Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
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
93 changes: 89 additions & 4 deletions src/clap/streaming.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 => {
Expand Down Expand Up @@ -554,6 +555,90 @@ mod tests {
);
}

#[test]
fn short_one_optional() {
let params: [clap::Param<u8>; 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 = &params[0];
let c = &params[1];
let pos = &params[2];

test_no_err(
&params,
&[
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<u8>; 4] = [
Expand Down
2 changes: 1 addition & 1 deletion src/install/PackageManager/CommandLineArguments.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ const BACKEND_PARAM: ParamType = clap::param!(
);

const SHARED_HEAD_PARAMS: &[ParamType] = &[
clap::param!("-c, --config <STR>? Specify path to config file (bunfig.toml)"),
clap::param!("-c, --config <STR> Specify path to config file (bunfig.toml)"),
clap::param!("-y, --yarn Write a yarn.lock file (yarn v1)"),
];

Expand Down
2 changes: 1 addition & 1 deletion src/runtime/cli/Arguments.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ const BASE_PARAMS_: &[ParamType] = concat_params!(
"--cwd <STR> Absolute path to resolve files & entry points from. This just changes the process' cwd."
),
parse_param!(
"-c, --config <PATH>? Specify path to Bun config file. Default <d>$cwd<r>/bunfig.toml"
"-c, --config <PATH> Specify path to Bun config file. Default <d>$cwd<r>/bunfig.toml"
),
parse_param!("-h, --help Display this menu and exit"),
],
Expand Down
3 changes: 1 addition & 2 deletions test/cli/install/bun-run.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"] : []),
Expand Down
99 changes: 99 additions & 0 deletions test/config/bunfig/config-flag.test.ts
Original file line number Diff line number Diff line change
@@ -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);
});
});
Comment thread
claude[bot] marked this conversation as resolved.
Loading