From 0d0691b7bad87763d2e0ad1a0ae90f5e3591c663 Mon Sep 17 00:00:00 2001 From: Gabriel Schneider Date: Thu, 3 Sep 2026 16:19:23 -0300 Subject: pipe: a filter that fails says so, and `| head -1` stops failing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two things made the selection pipe feel like it had never worked. It runs — test/snapshots/pipe.snap drives the real binary through a pty and filters `alpha beta` to `ALPHA BETA` — but it had no way to tell you when it did not, and one of its failure conditions was not a failure at all. IT NOW SAYS WHY. `runOne` read the command's stderr into memory and freed it two lines later, unread; every caller answered a failed filter with a bare `return`; `pipeResponse` had eight more silent exits under that. So `| trr` (a typo), `| grep nomatch` (exit 1), `| jq .` on bad JSON — all did nothing, said nothing, and left the text alone with no way to find out why. The runner carries a `Failure` home instead: which selection, what became of the command, and its own stderr. The core turns that into an `+Errors` buffer — acme's name for output that came from the program rather than from a word anybody clicked: | trr exit status 127 sh: line 1: trr: command not found An output buffer rather than the message row because the useful half of a shell failure is the text the shell wrote, and a 256-byte row would keep the label and throw away the reason. Focus stays with the file: `openRead` moves `p.active` to what it opens, which is right for a Grep you asked to read and wrong for a report you did not — you want to fix the command and press `|` again. A host with no `pull_pipe` at all (the detached daemon, the browser, the board) now says that too, instead of answering failure into the void. `| head -1` NOW WORKS. `writer_context.ok` was part of the success condition, so a command that stopped reading its stdin failed the filter even though it had done exactly its job: `head` takes the line it wants and closes the pipe, the write gets EPIPE, and a selection bigger than the 64 KiB pipe buffer was enough to trigger it. helix joins its input task and ignores the result for this reason; the exit status is the whole verdict. Also reported rather than swallowed: the ten-second timeout, the output ceilings, and a file edited while the filter ran — one keystroke during a slow command used to discard the result in a way indistinguishable from the filter doing nothing. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_016Q4RATpafkwahrovHQLKRf --- src/pardes.zig | 127 ++++++++++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 122 insertions(+), 5 deletions(-) (limited to 'src/pardes.zig') 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; -- cgit v1.3