diff options
Diffstat (limited to 'src')
| -rw-r--r-- | src/gui/gui.zig | 1 | ||||
| -rw-r--r-- | src/macos.zig | 1 | ||||
| -rw-r--r-- | src/output_pane.zig | 8 | ||||
| -rw-r--r-- | src/pardes.zig | 127 | ||||
| -rw-r--r-- | src/selection_pipe.zig | 159 | ||||
| -rw-r--r-- | src/tty/tty.zig | 1 |
6 files changed, 266 insertions, 31 deletions
diff --git a/src/gui/gui.zig b/src/gui/gui.zig index a87eebeb..b31fb5f6 100644 --- a/src/gui/gui.zig +++ b/src/gui/gui.zig @@ -3624,6 +3624,7 @@ const Shell = struct { .id = response.id, .success = response.success, .outputs = response.outputs, + .failure = response.failure, } }); response.deinit(s.gpa); s.saw_event = true; diff --git a/src/macos.zig b/src/macos.zig index 9d4f6039..c50d64cf 100644 --- a/src/macos.zig +++ b/src/macos.zig @@ -1418,6 +1418,7 @@ fn drainInbox(st: *State) bool { .id = value.id, .success = value.success, .outputs = value.outputs, + .failure = value.failure, } }); st.pipe_tasks.finish(st.io, value.id); }, diff --git a/src/output_pane.zig b/src/output_pane.zig index 1850fa92..288c3e43 100644 --- a/src/output_pane.zig +++ b/src/output_pane.zig @@ -572,6 +572,14 @@ pub fn openEffectCode(p: *Pardes, id: usize, argument: []const u8) !void { /// /// `content` is gpa-owned: adopted by the buffer, or freed here when there is /// nowhere to put it. +/// Put a report in this directory's `+Errors` buffer — acme's own name for +/// output that came from the PROGRAM rather than from a word somebody clicked. +/// Refills the one already open rather than stacking a twin, which is what a +/// second failed filter wants: the newest reason is the one being read. +pub fn openErrors(p: *Pardes, id: usize, content: []u8) !void { + return openRead(p, id, .errors, "", content); +} + fn openRead(p: *Pardes, id: usize, from: Origin, arg: []const u8, content: []u8) !void { errdefer p.gpa.free(content); const pane = p.panes[id] orelse return error.MissingPane; diff --git a/src/pardes.zig b/src/pardes.zig index 00226f3c..edff0ec7 100644 --- a/src/pardes.zig +++ b/src/pardes.zig @@ -784,6 +784,52 @@ test "selection pipe failure and stale completion never mutate the file" { try std.testing.expectEqual(@as(usize, 0), pane.file.?.undo_len); } +test "a failed filter opens an errors buffer carrying the command's own words" { + const gpa = std.testing.allocator; + const p = try Pardes.init(gpa, .{ .tty_only = true, .cols = 100, .rows = 30 }); + defer p.deinit(); + while (p.nextEffect()) |_| {} + const pane = try p.hxOpenFileContent("abc\n"); + pane.cur_col = 2; + pane.vsel = .{ .active = true, .row = 0, .col = 0, .explicit = true }; + + p.update(.{ .key = .{ .cp = '|' } }); + for ("trr") |c| p.update(.{ .key = .{ .cp = c, .text = &.{c} } }); + p.update(.{ .key = .{ .cp = Key.enter } }); + const id = nextPipeEffect(p) orelse return error.MissingPipeEffect; + + // The shape a real runner brings home for `| trr`: nonzero exit, and the + // shell's own sentence about it. + var stderr = "sh: line 1: trr: command not found\n".*; + p.update(.{ .pipe_resp = .{ + .id = id, + .success = false, + .outputs = &.{}, + .failure = .{ .kind = .exit, .code = 127, .stderr = &stderr }, + } }); + + // The text is untouched — a failed filter is not an edit... + try std.testing.expectEqualSlices(u8, "abc\n", pane.file.?.content); + try std.testing.expectEqual(@as(usize, 0), pane.file.?.undo_len); + // ...and the cursor did not go anywhere, so `|` again edits the same file. + try std.testing.expectEqual(@as(usize, 0), p.active); + + // ...but the reason is now READABLE, in an +Errors buffer: the command as + // typed, what became of it, and what the shell said. Every one of those + // three used to be dropped on the floor. + var found: ?[]const u8 = null; + for (p.panes) |slot| { + const q = slot orelse continue; + const qf = q.file orelse continue; + const o = qf.output orelse continue; + if (std.meta.activeTag(o.from) == .errors) found = qf.content; + } + const report = found orelse return error.NoErrorsBuffer; + try std.testing.expect(std.mem.indexOf(u8, report, "| trr") != null); + try std.testing.expect(std.mem.indexOf(u8, report, "exit status 127") != null); + try std.testing.expect(std.mem.indexOf(u8, report, "command not found") != null); +} + test "selection pipe rejects a reused pane slot and a superseded request" { const gpa = std.testing.allocator; const p = try Pardes.init(gpa, .{ .tty_only = true }); @@ -4046,7 +4092,15 @@ pub const Event = union(enum) { /// A selection-pipe worker finished. Every output is borrowed for this /// update only; success is atomic, so a failed/nonzero invocation carries /// no usable outputs and changes nothing. - pipe_resp: struct { id: u32, success: bool, outputs: []const []const u8 }, + /// `failure` is borrowed for this update like `outputs`, and is what the + /// core turns into an `+Errors` pane. Null with `success = false` means + /// nobody ever ran it — a host with no `pull_pipe` at all. + pipe_resp: struct { + id: u32, + success: bool, + outputs: []const []const u8, + failure: ?selection_pipe.Failure = null, + }, /// a file the shell was asked to watch changed on disk; `bytes` are the /// exact snapshot the host hashed, borrowed for this call like `output`. /// Text panes adopt a copy; PDF panes reopen the path so MuPDF owns its @@ -7525,7 +7579,7 @@ pub const Pardes = struct { }, .eof => |e| p.removePane(e.pane), .lsp_resp => |r| p.lspResponse(r.id, r.rows), - .pipe_resp => |r| p.pipeResponse(r.id, r.success, r.outputs), + .pipe_resp => |r| p.pipeResponse(r.id, r.success, r.outputs, r.failure), .file_changed => |fc| _ = p.applyWatchedFileChanged(fc.pane, fc.bytes), .key => |key| p.handleKey(key), .mouse => |m| { @@ -10365,17 +10419,80 @@ pub const Pardes = struct { return null; } - fn pipeResponse(p: *Pardes, id: u32, success: bool, outputs: []const []const u8) void { + /// Put a failed filter where it can be READ: the command, which selection + /// it was, what became of it, and the command's own stderr underneath. + /// + /// An `+Errors` pane rather than the message row, because a message row is + /// 256 bytes and one line, and the useful half of a shell failure is the + /// text the shell wrote — `sh: line 1: trr: command not found`, a compiler + /// diagnostic, a `jq` parse error with a column in it. Truncating that to + /// fit a row would throw away the reason and keep the label. + fn pipeFailed(p: *Pardes, wait: *const PendingPipe, failure: ?selection_pipe.Failure) void { + var out: std.Io.Writer.Allocating = .init(p.gpa); + defer out.deinit(); + const w = &out.writer; + w.print("| {s}\n\n", .{wait.command}) catch return; + if (failure) |fail| { + if (wait.inputs.len > 1) + w.print("selection {d} of {d}: ", .{ fail.index + 1, wait.inputs.len }) catch return; + switch (fail.kind) { + .exit => w.print("exit status {d}\n", .{fail.code}) catch return, + .signal => w.writeAll("killed by a signal\n") catch return, + .timeout => w.print( + "still running after {d} seconds, and stopped\n", + .{selection_pipe.command_timeout_seconds}, + ) catch return, + .too_large => w.writeAll("produced more output than a filter may return\n") catch return, + .spawn => w.writeAll("could not be started\n") catch return, + .io => w.writeAll("could not be read\n") catch return, + } + if (fail.stderr.len > 0) w.print("\n{s}", .{fail.stderr}) catch return; + } else { + // No diagnosis at all: nobody ran it. The detached daemon, the + // browser and the board all leave `pull_pipe` null on purpose. + w.writeAll("this session cannot run filters\n") catch return; + } + const content = out.toOwnedSlice() catch return; + // FOCUS STAYS WITH THE TEXT. `openRead` moves `p.active` to the buffer + // it opens, which is right for a `Grep` you asked to read and wrong for + // a report you did not: a failed filter should put the reason on screen + // and leave the cursor in the file you were filtering, ready to fix the + // command and press `|` again. + const was = p.active; + output_pane.openErrors(p, wait.pane, content) catch |err| { + p.gpa.free(content); + // Nowhere to put the report is itself worth one line. + p.reportError(wait.pane, "pipe", err); + return; + }; + if (p.panes[was] != null) p.active = was; + } + + fn pipeResponse( + p: *Pardes, + id: u32, + success: bool, + outputs: []const []const u8, + failure: ?selection_pipe.Failure, + ) void { + // A SUPERSEDED OR UNKNOWN id is the one silence worth keeping: it is + // the answer to a question nobody is still asking. if (p.pipe_wait == null or p.pipe_wait.?.id != id) return; var wait = p.pipe_wait.?; p.pipe_wait = null; defer wait.deinit(p.gpa); - if (!success or outputs.len != wait.inputs.len) return; + + if (!success or outputs.len != wait.inputs.len) return p.pipeFailed(&wait, failure); const pane = p.panes[wait.pane] orelse return; if (pane.serial != wait.serial) return; const f = if (pane.file) |*file| file else return; - if (!output_pane.fileTraits(f.output).saves or f.revision != wait.revision) return; + if (!output_pane.fileTraits(f.output).saves) return; + // THE FILE MOVED UNDER THE FILTER. One keystroke during a ten-second + // command was enough to discard the whole result in silence, which is + // indistinguishable from the filter having done nothing at all. + if (f.revision != wait.revision) + return p.reportError(wait.pane, "pipe", error.FileChangedWhileFiltering); var total_output: usize = 0; var removed: usize = 0; diff --git a/src/selection_pipe.zig b/src/selection_pipe.zig index c1104010..8e721751 100644 --- a/src/selection_pipe.zig +++ b/src/selection_pipe.zig @@ -65,19 +65,60 @@ pub const Job = struct { } }; -/// One worker answer. `outputs` owns each slice; a failure owns an empty list. +/// WHY a filter produced nothing, carried home so somebody can be told. +/// +/// Until this existed the runner read the command's stderr into memory and +/// then FREED IT UNREAD — the one artifact that explains a failure, discarded +/// two lines after it arrived — and every caller answered a failed filter with +/// a bare `return`. `| trr a-z A-Z` (a typo), `| grep nomatch` (exit 1), +/// `| jq .` on bad JSON: all of them did nothing, said nothing, and left the +/// text alone with no way to find out why. +pub const Failure = struct { + pub const Kind = enum { + /// The command never started: no `/bin/sh`, a cwd that is gone, a NUL + /// in the command, a fork that failed. + spawn, + /// Still running at `command_timeout_seconds`. + timeout, + /// Past `max_stdout_bytes` / `max_stderr_bytes` / the job total. + too_large, + /// Ran, and exited nonzero. `code` says which. + exit, + /// Killed by a signal. + signal, + /// A read, a write or an allocation failed under us. + io, + }; + + kind: Kind = .io, + /// Which selection this was, so a report over several cursors can say. + index: u32 = 0, + /// The exit status, when `kind` is `.exit`. + code: u8 = 0, + /// stderr exactly as the command wrote it, OWNED by the response. Empty + /// when the command said nothing, which is why `kind` and `code` exist. + stderr: []u8 = &.{}, +}; + +/// One worker answer. `outputs` owns each slice; a failure owns an empty list +/// and, usually, the command's own account of itself in `failure`. pub const Response = struct { id: u32, success: bool, outputs: [][]u8, + failure: ?Failure = null, pub fn deinit(response: *Response, gpa: std.mem.Allocator) void { for (response.outputs) |output| gpa.free(output); if (response.outputs.len > 0) gpa.free(response.outputs); + if (response.failure) |f| if (f.stderr.len > 0) gpa.free(f.stderr); response.* = undefined; } }; +/// What one invocation came to: the bytes, or the reason there are none. +pub const Outcome = union(enum) { ok: []u8, failed: Failure }; + /// The in-flight set a host keeps while pipes run off its loop. /// /// tty.zig and gui.zig each had this verbatim — same `finish` walk, same @@ -151,9 +192,14 @@ pub fn runOne( command: []const u8, cwd: []const u8, input: []const u8, -) ?[]u8 { +) Outcome { + const fail = struct { + fn k(kind: Failure.Kind) Outcome { + return .{ .failed = .{ .kind = kind } }; + } + }; if (std.mem.indexOfScalar(u8, command, 0) != null or - std.mem.indexOfScalar(u8, cwd, 0) != null) return null; + std.mem.indexOfScalar(u8, cwd, 0) != null) return fail.k(.spawn); var child = std.process.spawn(io, .{ .argv = &.{ "/bin/sh", "-c", command }, @@ -161,7 +207,7 @@ pub fn runOne( .stdin = .pipe, .stdout = .pipe, .stderr = .pipe, - }) catch return null; + }) catch return fail.k(.spawn); var writer_context: WriterContext = .{ .io = io, @@ -172,7 +218,7 @@ pub fn runOne( var writer: ?std.Thread = std.Thread.spawn(.{}, writeInput, .{&writer_context}) catch { writer_context.file.close(io); child.kill(io); - return null; + return fail.k(.spawn); }; // On every early return kill first, unblocking a command which never read // stdin, then join the short-lived writer before its borrowed input dies. @@ -192,33 +238,49 @@ pub fn runOne( } }).toDeadline(io); while (multi_reader.fill(64, deadline)) |_| { if (stdout_reader.buffered().len > max_stdout_bytes or - stderr_reader.buffered().len > max_stderr_bytes) return null; + stderr_reader.buffered().len > max_stderr_bytes) return fail.k(.too_large); } else |err| switch (err) { error.EndOfStream => {}, - else => return null, + // The deadline is the only one of these a human is likely to cause, + // and it is the one they are least able to guess at: ten seconds of + // nothing used to be followed by nothing. + error.Timeout => return fail.k(.timeout), + else => return fail.k(.io), } if (stdout_reader.buffered().len > max_stdout_bytes or - stderr_reader.buffered().len > max_stderr_bytes) return null; - multi_reader.checkAnyError() catch return null; - const term = child.wait(io) catch return null; + stderr_reader.buffered().len > max_stderr_bytes) return fail.k(.too_large); + multi_reader.checkAnyError() catch return fail.k(.io); + const term = child.wait(io) catch return fail.k(.io); writer.?.join(); writer = null; - const stdout = multi_reader.toOwnedSlice(0) catch return null; + const stdout = multi_reader.toOwnedSlice(0) catch return fail.k(.io); const stderr = multi_reader.toOwnedSlice(1) catch { gpa.free(stdout); - return null; - }; - defer gpa.free(stderr); - const exited_zero = switch (term) { - .exited => |code| code == 0, - else => false, + return fail.k(.io); }; - if (!writer_context.ok or !exited_zero) { - gpa.free(stdout); - return null; + // stderr is NOT freed here any more. It is the command's own account of + // what went wrong, and it goes home with the failure. + errdefer gpa.free(stderr); + + // A STDIN WRITE THAT ENDED EARLY IS NOT A FAILURE. `writer_context.ok` was + // part of this condition, so `| head -1` over a selection bigger than the + // pipe buffer failed — the command closed stdin after the line it wanted, + // the write got EPIPE, and a filter that had done exactly its job reported + // nothing. helix joins its input task and ignores the result for this + // reason; the exit status is the whole verdict. + switch (term) { + .exited => |code| if (code != 0) { + gpa.free(stdout); + return .{ .failed = .{ .kind = .exit, .code = code, .stderr = stderr } }; + }, + else => { + gpa.free(stdout); + return .{ .failed = .{ .kind = .signal, .stderr = stderr } }; + }, } - return stdout; + gpa.free(stderr); + return .{ .ok = stdout }; } /// Invoke the command independently for every selection. The response is all @@ -230,7 +292,17 @@ pub fn runJob(gpa: std.mem.Allocator, io: std.Io, job: *const Job) Response { var made: usize = 0; var total: usize = 0; for (job.inputs, 0..) |input, i| { - const output = runOne(gpa, io, job.command, job.cwd, input) orelse break; + const output = switch (runOne(gpa, io, job.command, job.cwd, input)) { + .ok => |bytes| bytes, + .failed => |f| { + // WHICH selection, because with several cursors "it failed" is + // not enough to go looking with. + var owned = f; + owned.index = @intCast(i); + response.failure = owned; + break; + }, + }; if (std.math.add(usize, total, output.len) catch null) |next_total| { if (next_total <= max_total_stdout_bytes) { outputs[i] = output; @@ -240,6 +312,7 @@ pub fn runJob(gpa: std.mem.Allocator, io: std.Io, job: *const Job) Response { } } gpa.free(output); + response.failure = .{ .kind = .too_large, .index = @intCast(i) }; break; } if (made != job.inputs.len) { @@ -252,12 +325,46 @@ pub fn runJob(gpa: std.mem.Allocator, io: std.Io, job: *const Job) Response { return response; } -test "native pipe runner preserves stdin/stdout bytes and rejects failure" { +test "native pipe runner preserves stdin/stdout bytes and reports how it failed" { const gpa = std.testing.allocator; const io = std.testing.io; - const output = runOne(gpa, io, "printf 'prefix:'; cat; printf '\\n'", "/tmp", "a\x00b\n") orelse - return error.PipeCommandFailed; + const output = switch (runOne(gpa, io, "printf 'prefix:'; cat; printf '\\n'", "/tmp", "a\x00b\n")) { + .ok => |bytes| bytes, + .failed => return error.PipeCommandFailed, + }; defer gpa.free(output); try std.testing.expectEqualSlices(u8, "prefix:a\x00b\n\n", output); - try std.testing.expect(runOne(gpa, io, "printf ignored; exit 7", "/tmp", "") == null); + + // A NONZERO EXIT COMES HOME WITH ITS REASON. The status and the command's + // own stderr are the whole of what a human needs to fix a typo'd filter, + // and both used to be freed on the floor. + switch (runOne(gpa, io, "echo trouble >&2; exit 7", "/tmp", "")) { + .ok => |bytes| { + gpa.free(bytes); + return error.PipeShouldHaveFailed; + }, + .failed => |f| { + defer gpa.free(f.stderr); + try std.testing.expectEqual(Failure.Kind.exit, f.kind); + try std.testing.expectEqual(@as(u8, 7), f.code); + try std.testing.expectEqualSlices(u8, "trouble\n", f.stderr); + }, + } + + // ...and a command that stops reading its stdin SUCCEEDS. `| head -1` over + // a selection bigger than the pipe buffer takes the line it wanted and + // closes the pipe; the write gets EPIPE, and that used to fail the filter + // even though it had done exactly its job. + const big = try gpa.alloc(u8, 512 * 1024); + defer gpa.free(big); + @memset(big, 'x'); + big[0] = 'a'; + big[1] = '\n'; + switch (runOne(gpa, io, "head -1", "/tmp", big)) { + .ok => |bytes| { + defer gpa.free(bytes); + try std.testing.expectEqualSlices(u8, "a\n", bytes); + }, + .failed => return error.EarlyStdinCloseShouldNotFail, + } } diff --git a/src/tty/tty.zig b/src/tty/tty.zig index de24f289..ff86ef88 100644 --- a/src/tty/tty.zig +++ b/src/tty/tty.zig @@ -1160,6 +1160,7 @@ const Shell = struct { .id = response.id, .success = response.success, .outputs = response.outputs, + .failure = response.failure, } }); response.deinit(s.gpa); s.pipe_tasks.finish(s.io, response_value.id); |