Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: short params parsing within zig-clap fork #14845

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
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
2 changes: 2 additions & 0 deletions src/deps/zig-clap/clap.zig
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ const Output = @import("../../output.zig");

pub const args = @import("clap/args.zig");

pub const default_assignment_separators = "=";

test "clap" {
testing.refAllDecls(@This());
}
Expand Down
36 changes: 26 additions & 10 deletions src/deps/zig-clap/clap/streaming.zig
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,12 @@ pub fn Arg(comptime Id: type) type {
};
}

pub const ArgError = error{
DoesntTakeValue,
MissingValue,
InvalidArgument,
};

/// A command line argument parser which, given an ArgIterator, will parse arguments according
/// to the params. StreamingClap parses in an iterating manner, so you have to use a loop together with
/// StreamingClap.next to parse all the arguments of your program.
Expand All @@ -41,19 +47,18 @@ pub fn StreamingClap(comptime Id: type, comptime ArgIterator: type) type {
};
};

const ArgError = error{ DoesntTakeValue, MissingValue, InvalidArgument };

params: []const clap.Param(Id),
iter: *ArgIterator,
state: State = .normal,
positional: ?*const clap.Param(Id) = null,
diagnostic: ?*clap.Diagnostic = null,
assignment_separators: []const u8 = clap.default_assignment_separators,

/// Get the next Arg that matches a Param.
pub fn next(parser: *@This()) ArgError!?Arg(Id) {
switch (parser.state) {
.normal => return try parser.normal(),
.chaining => |state| return try parser.chainging(state),
.chaining => |state| return try parser.chaining(state),
.rest_are_positional => {
const param = parser.positionalParam() orelse unreachable;
const value = parser.iter.next() orelse return null;
Expand Down Expand Up @@ -118,7 +123,7 @@ pub fn StreamingClap(comptime Id: type, comptime ArgIterator: type) type {
}
return null;
},
.short => return try parser.chainging(.{
.short => return try parser.chaining(.{
.arg = arg,
.index = 0,
}),
Expand All @@ -140,7 +145,7 @@ pub fn StreamingClap(comptime Id: type, comptime ArgIterator: type) type {
}
}

fn chainging(parser: *@This(), state: State.Chaining) ArgError!?Arg(Id) {
fn chaining(parser: *@This(), state: State.Chaining) ArgError!?Arg(Id) {
const arg = state.arg;
const index = state.index;
const next_index = index + 1;
Expand All @@ -164,9 +169,13 @@ pub fn StreamingClap(comptime Id: type, comptime ArgIterator: type) type {
}
}

const next_is_eql = if (next_index < arg.len) arg[next_index] == '=' else false;
if (param.takes_value == .none or param.takes_value == .one_optional) {
if (next_is_eql and param.takes_value == .none)
const next_is_separator = if (next_index < arg.len)
std.mem.indexOfScalar(u8, parser.assignment_separators, arg[next_index]) != null
else
false;

if (param.takes_value == .none) {
if (next_is_separator)
return parser.err(arg, .{ .short = short }, error.DoesntTakeValue);
return Arg(Id){ .param = param };
}
Expand All @@ -178,13 +187,20 @@ pub fn StreamingClap(comptime Id: type, comptime ArgIterator: type) type {
return Arg(Id){ .param = param, .value = value };
}

if (next_is_eql)
if (next_is_separator)
return Arg(Id){ .param = param, .value = arg[next_index + 1 ..] };

return Arg(Id){ .param = param, .value = arg[next_index..] };
}

return parser.err(arg, .{ .short = arg[index] }, error.InvalidArgument);
if (warn_on_unrecognized_flag) {
Output.warn("unrecognized bun flag: -{c}, continuing to parse and evaluate flag downstream...\n", .{arg[index]});
Output.flush();
}

// Continue parsing after unrecognized flag
parser.state = .normal;
return parser.next();
Comment on lines -187 to +203
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the main change of this PR

}

fn positionalParam(parser: *@This()) ?*const clap.Param(Id) {
Expand Down
48 changes: 48 additions & 0 deletions test/regression/issue/07114.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
import { expect, test } from "bun:test";
import { bunEnv, bunExe, tempDirWithFiles } from "harness";

test("short flags should be properly parsed", () => {
const dir = tempDirWithFiles("07114", {
"package.json": JSON.stringify({
name: "short-flags-test",
version: "0.0.0",
}),
});

// Test single short flag
const singleFlag = Bun.spawnSync({
cmd: [bunExe(), "-t"], // as in `bun create expo ./awesome-project -t tabs`
cwd: dir,
env: bunEnv,
stderr: "pipe",
});
expect(singleFlag.stderr.toString().toLowerCase()).not.toContain("invalid argument '-t'");

// Test multiple combined short flags
const multipleFlags = Bun.spawnSync({
cmd: [bunExe(), "-ab"],
cwd: dir,
env: bunEnv,
stderr: "pipe",
});
expect(multipleFlags.stderr.toString().toLowerCase()).not.toContain("invalid argument");
expect(multipleFlags.stderr.toString().toLowerCase()).not.toContain("requires a value but none was supplied");

// Test short flag with value
const flagWithValue = Bun.spawnSync({
cmd: [bunExe(), "-p", "3000"],
cwd: dir,
env: bunEnv,
stderr: "pipe",
});
expect(flagWithValue.stderr.toString().toLowerCase()).not.toContain("invalid argument '-p'");

// Test short flag that requires a value but none was supplied
const flagWithoutValue = Bun.spawnSync({
cmd: [bunExe(), "-p"],
cwd: dir,
env: bunEnv,
stderr: "pipe",
});
expect(flagWithoutValue.stderr.toString().toLowerCase()).toContain("requires a value but none was supplied");
});