shell: more builtin commands #9908
Conversation
|
❌ @nektro 4 files with test failures on linux-x64: |
|
❌ @nektro 2 files with test failures on linux-x64-baseline: |
|
❌ @nektro 1 files with test failures on bun-darwin-aarch64:
|
|
❌ @nektro 7 files with test failures on bun-windows-x86_64-haswell
|
|
❌ @nektro 3 files with test failures on bun-darwin-x64:
|
| @@ -5174,27 +5157,25 @@ pub const Interpreter = struct { | |||
| } | |||
|
|
|||
| /// **WARNING** You should make sure that stdout/stderr does not need IO (e.g. `.needsIO(.stderr)` is false before caling `.writeNoIO(.stderr, buf)`) | |||
| pub fn writeNoIO(this: *Builtin, comptime io_kind: @Type(.EnumLiteral), buf: []const u8) usize { | |||
| pub fn writeNoIO(this: *Builtin, comptime io_kind: @Type(.EnumLiteral), buf: []const u8) Maybe(usize) { | |||
There was a problem hiding this comment.
Unfortunately, because the previous value was a usize, almost every call to (about 34 of them) writeNoIO() discarded the value (e.g. _ = writeNoIO())
I think we should revert this change and do it in another PR as it will require a lot of changes to handle the error everywhere
There was a problem hiding this comment.
I think its okay if most ignore it, but in yes it was imperative so that it knows when to stop
| const str = std.fmt.allocPrint(arena.allocator(), "{d}", .{current}) catch bun.outOfMemory(); | ||
| defer _ = arena.reset(.retain_capacity); |
There was a problem hiding this comment.
What about printing to this.buf.writer() instead?
There was a problem hiding this comment.
buf is only used if this.bltn.stdout.needsIO()
| this.buf.appendSlice(bun.default_allocator, msg) catch bun.outOfMemory(); | ||
| return Maybe(void).success; | ||
| } | ||
| const res = this.bltn.writeNoIO(.stdout, msg); |
There was a problem hiding this comment.
Suffers from same problem as Yes, example:
await Promise.all([$`seq 9999`, $`cat src/js_parser.zig`]);There was a problem hiding this comment.
this may be a latent bug from elsewhere since bun-debug exec 'seq 9999' exits near instantly as do bun exec 'cat file'
|
it might also be a good idea to update the website (built-in commands) I see only 6 commands as implemented on the website but in reality it's closer to 15 with this PR |
#9716 progress
added yes, seq, dirname, basename.