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

renanmav · 2024-10-26T16:36:02Z

What does this PR do?

This pull request fixes an issue with parsing short parameters (e.g., -t) in Bun's fork of zig-clap (upstream repo). The bug was causing errors when parsing single short flags, as reported in #7114. The fix ensures that short flags are correctly recognized and processed while maintaining compatibility with existing behavior.

Note

I'm not super familiar with the Zig language, so I pushed this fix with the help of Cursor AI.

Apologies for any mistakes, appreciate your review.

Changes Made

How did you verify your code works?

I've built the package locally with code changes, and did testing using these commands:

$ bun run build
$ ./build/debug/bun-debug create expo ./awesome-project -t tabs

Output:

Before

[SYS] read(3[/Users/renanmav/renanmav/bun/build/debug/bun-debug], 4096) = 4096 (4.244ms)
[SYS] close(3[/Users/renanmav/renanmav/bun/build/debug/bun-debug])
error: Invalid Argument '-t'
note: Release build will not have this trace by default:
/Users/renanmav/renanmav/bun/src/deps/zig-clap/clap/streaming.zig:231:13: 0x10285137b in err__anon_122110 (bun-debug)
            return _err;
            ^
/Users/renanmav/renanmav/bun/src/deps/zig-clap/clap/streaming.zig:187:13: 0x1022bc2cf in chainging (bun-debug)
            return parser.err(arg, .{ .short = arg[index] }, error.InvalidArgument);
            ^
/Users/renanmav/renanmav/bun/src/deps/zig-clap/clap/streaming.zig:121:34: 0x1022bb33b in normal (bun-debug)
                .short => return try parser.chainging(.{
                                 ^
/Users/renanmav/renanmav/bun/src/deps/zig-clap/clap/streaming.zig:55:35: 0x101e359cf in next (bun-debug)
                .normal => return try parser.normal(),
                                  ^
/Users/renanmav/renanmav/bun/src/deps/zig-clap/clap/comptime.zig:76:20: 0x1020e51b7 in parse__anon_87277 (bun-debug)
            while (try stream.next()) |arg| {
                   ^
/Users/renanmav/renanmav/bun/src/deps/zig-clap/clap.zig:333:12: 0x1020e58eb in parseEx__anon_87276 (bun-debug)
    return try Clap.parse(iter, opt);
           ^
/Users/renanmav/renanmav/bun/src/deps/zig-clap/clap.zig:316:16: 0x1020e5ac7 in parse__anon_87275 (bun-debug)
    res.clap = try parseEx(Id, params, &iter, .{
               ^
/Users/renanmav/renanmav/bun/src/cli/create_command.zig:216:13: 0x1020e5d37 in parse (bun-debug)
            return err;
            ^
/Users/renanmav/renanmav/bun/src/cli/create_command.zig:1680:32: 0x1020e64a3 in extractInfo (bun-debug)
        const create_options = try CreateOptions.parse(ctx);
                               ^
/Users/renanmav/renanmav/bun/src/cli.zig:1972:45: 0x102108fff in start (bun-debug)
                const create_command_info = try CreateCommand.extractInfo(ctx);

After

[SYS] read(3[/Users/renanmav/renanmav/bun/build/debug/bun-debug], 4096) = 4096 (0.108ms)
[SYS] close(3[/Users/renanmav/renanmav/bun/build/debug/bun-debug])
[bunx] initial_bin_name: create-expo
[SYS] openat(7[/Users/renanmav/renanmav/bun], package.json) = 8
[fs] openat(7[/Users/renanmav/renanmav/bun], /Users/renanmav/renanmav/bun/package.json) = 8[/Users/renanmav/renanmav/bun/package.json]
[SYS] close(8[/Users/renanmav/renanmav/bun/package.json])
[alloc] new(PackageJSON) = src.resolver.package_json.PackageJSON@11ef04080
[bunx] package_fmt: create-expo@latest
[bunx] install_param: create-expo@latest
[bunx] result_package_name: create-expo
[bunx] bunx_cache_dir: /private/tmp/bunx-501-create-expo@latest
[SYS] openat(7[/Users/renanmav/renanmav/bun], node_modules/create-expo/package.json) = -1
[SYS] openat(-2, /private/tmp/bunx-501-create-expo@latest/package.json) = -1
[bunx] installing package: /Users/renanmav/renanmav/bun/build/debug/bun-debug add create-expo@latest --no-summary --no-cache --force
[SYS] posix_spawn(/Users/renanmav/renanmav/bun/build/debug/bun-debug) = 0 (74079)
[SYS] read(3[/Users/renanmav/renanmav/bun/build/debug/bun-debug], 4096) = 4096 (0.035ms)
[SYS] close(3[/Users/renanmav/renanmav/bun/build/debug/bun-debug])
[SYS] openat(-2, /Users/renanmav/.bunfig.toml) = -1
[SYS] openat(-2, /private/tmp/bunx-501-create-expo@latest/bunfig.toml) = -1
[SYS] openat(-2, /private/tmp/bunx-501-create-expo@latest) = 4
[SYS] openat(-2, /Users/renanmav/.npmrc) = 5
[SYS] fstat(5[/Users/renanmav/.npmrc]) = 0
[SYS] close(5[/Users/renanmav/.npmrc])
[SYS] openat(-2, .npmrc) = -1
[SYS] openat(-2, /private/tmp/bunx-501-create-expo@latest/package.json) = 7
[SYS] fstat(7[/private/tmp/bunx-501-create-expo@latest/package.json]) = 0
[SYS] close(7[/private/tmp/bunx-501-create-expo@latest/package.json])
[alloc] new(Request) = src.bun.js.api.bun.dns_resolver.InternalDNS.Request@144804080
[SYS] FilePoll.init(0x144904080, generation_number=1, fd=4355)
[SYS] register: FilePoll(0x144904080, generation_number=1) machport (4355)
[SYS] openat(-2, bun.lockb) = -1
[SYS] openat(-2, package-lock.json) = -1
[OverrideMap] parsed 0 overrides
[OverrideMap] looking up override for b22dbb75aa0c7f9c
  🔍 Resolving [1/1] [alloc] new(ThreadlocalAsyncHTTP) = src.http.ThreadlocalAsyncHTTP@146808200
[uws] connect(registry.npmjs.org, 443)
[fetch] Processed 1 tasks
  🔍 Resolving [1/1] [SYS] onKQueueEvent: FilePoll(fd=4355, generation_number=1) = poll_machport | machport | one_shot | has_incremented_poll_count | was_ever_registered
[SYS] onUpdate kevent (fd: 4355) InternalDNSRequest
  🔍 Resolving [1/1] [fetch] Connected https://registry.npmjs.org/create-expo
[uws] us_socket_write(*src.deps.boringssl.translated.SSL@600002af0008, 314) = 314
[uws] us_socket_write(*src.deps.boringssl.translated.SSL@600002af0008, 0) = 0
[fetch] onData 449
[fetch] handleResponseMetadata: content_length is null and transfer_encoding src.http.Encoding.chunked
[fetch] onData 1369
[fetch] onData 2738
[fetch] onData 3348
[fetch] onData 1369
[fetch] onData 1369
[fetch] onData 2908
[fetch] onData 2473
[fetch] Decompressing 15543 bytes
[alloc] new(BrotliReaderArrayList) = src.brotli.BrotliReaderArrayList@147c04080
[fetch] progressUpdate true
[alloc] destroy(BrotliReaderArrayList) = src.brotli.BrotliReaderArrayList@147c04080
[fetch] onAsyncHTTPCallback: 1.384s
[alloc] destroy(ThreadlocalAsyncHTTP) = src.http.ThreadlocalAsyncHTTP@146808200
  🔍 create-expo [2/2] [OverrideMap] looking up override for b22dbb75aa0c7f9c
[SYS] faccessat(8[/Users/renanmav/.bun/install/cache], [email protected]@@@1, O_RDONLY, 0) = 0
[SYS] openat(-2, node_modules) = -1
[SYS] openat(-2, node_modules) = 13
[SYS] renameat(14[/private/tmp/bunx-501-create-expo@latest/node_modules], create-expo, 14[/private/tmp/bunx-501-create-expo@latest/node_modules], .old-61CACBD352C0255A) = 2
[SYS] renameat(14[/private/tmp/bunx-501-create-expo@latest/node_modules], create-expo, 14[/private/tmp/bunx-501-create-expo@latest/node_modules], .old-A9B89845C06917DB) = 2
[SYS] openat([invalid_fd], /private/tmp/bunx-501-create-expo@latest/node_modules/create-expo/build/index.js) = 15
[SYS] read(15[/private/tmp/bunx-501-create-expo@latest/node_modules/create-expo/build/index.js], 1024) = 1024 (0.191ms)
[SYS] close(15[/private/tmp/bunx-501-create-expo@latest/node_modules/create-expo/build/index.js])
  🔒 Saving lockfile... [SYS] openat(-2, .lockb-0e3795f981cd3ebf.tmp) = 14
[SYS] write(14[/private/tmp/bunx-501-create-expo@latest/.lockb-0e3795f981cd3ebf.tmp], 1255) = 1255 (0.035ms)
[SYS] renameat(-2, .lockb-0e3795f981cd3ebf.tmp, -2, bun.lockb) = 0
[SYS] close(14[/private/tmp/bunx-501-create-expo@latest/bun.lockb])
[SYS] openat([invalid_fd], /private/tmp/bunx-501-create-expo@latest/package.json) = 14
✔ Downloaded and extracted project files.
> bun install
bun install v1.1.30 (7996d06b)

+ @babel/[email protected]
+ @types/[email protected] (v18.3.12 available)
+ [email protected]
+ [email protected]
+ [email protected]
+ [email protected] (v5.6.3 available)
+ @expo/[email protected]
+ @react-navigation/[email protected]
+ [email protected]
+ [email protected]
+ [email protected]
+ [email protected]
+ [email protected]
+ [email protected]
+ [email protected]
+ [email protected]
+ [email protected]
+ [email protected]
+ [email protected]
+ [email protected] (v3.16.1 available)
+ [email protected]
+ [email protected]
+ [email protected]

1134 packages installed [14.78s]

✅ Your project is ready!

To run your project, navigate to the directory and run one of the following bun commands.

- cd awesome-project
- bun run android
- bun run iOS
- bun run web

Jarred-Sumner · 2024-11-05T23:31:03Z

Can you add automated tests?

renanmav · 2024-11-06T16:31:43Z

@Jarred-Sumner of course, just rebased this branch against latest main.

How do I unblock CI to run?

renanmav · 2024-11-09T23:33:50Z

I was curious to see the diff between the zig-clap fork and upstream.

Check it out here: main...renanmav:bun:renan/sync-zig-clap

renanmav · 2024-11-24T05:29:19Z

src/deps/zig-clap/clap/streaming.zig

-            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();


This is the main change of this PR

renanmav force-pushed the renan/fix-short-params branch from c6660a5 to eec63e1 Compare November 6, 2024 16:27

renanmav force-pushed the renan/fix-short-params branch from 2233c05 to b97ff56 Compare November 9, 2024 23:07

renanmav force-pushed the renan/fix-short-params branch from 1302eaf to 3b611eb Compare November 20, 2024 18:07

renanmav added 10 commits November 20, 2024 15:08

fix: short params parsing within zig-clap fork

408ecbb

refactor: maintain compatibility and proper error handling

26c228d

test: push 7114 regression test

c16b7da

chore: fix typo chainging -> chaining

18e7b76

chore: remove unused parseShortFlag fn

65e2e07

test: pass stderr to lower case

f352367

test: amend test with flags that requires a value but none was supplied

e5ed6c7

chore: sync zig-clap fork with upstream

a4e416c

feat: enable warn_on_unrecognized_flag

4f5fb6d

feat: continue parsing after unrecognized short flag

78bb5f3

renanmav force-pushed the renan/fix-short-params branch from 3b611eb to 78bb5f3 Compare November 20, 2024 18:08

renanmav commented Nov 24, 2024

View reviewed changes

renanmav added 3 commits November 26, 2024 12:42

Merge branch 'main' into renan/fix-short-params

2dd69e8

Merge branch 'main' into renan/fix-short-params

519684c

Merge branch 'main' into renan/fix-short-params

77be373

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

renanmav commented Oct 26, 2024 •

edited

Loading

Jarred-Sumner commented Nov 5, 2024

renanmav commented Nov 6, 2024

renanmav commented Nov 9, 2024

renanmav Nov 24, 2024

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

Are you sure you want to change the base?

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

Conversation

renanmav commented Oct 26, 2024 • edited Loading

What does this PR do?

Changes Made

How did you verify your code works?

Jarred-Sumner commented Nov 5, 2024

renanmav commented Nov 6, 2024

renanmav commented Nov 9, 2024

renanmav Nov 24, 2024

Choose a reason for hiding this comment

renanmav commented Oct 26, 2024 •

edited

Loading