From c44894f53decbacc68c0e4e9d1118fc082bc63a0 Mon Sep 17 00:00:00 2001 From: Gabriel Schneider Date: Mon, 28 Sep 2026 10:02:33 -0300 Subject: A ctl write fails when a builtin it runs fails, and a missing required argument is refused before anything runs A ctl write failed only on a malformed line: Mount x reported its error in the editor while the write succeeded, and a bare Mount failed only as it ran, after earlier lines of the write. acme's ctl answers a command's error (editors/acme/xfid.c:700). Builtins now declare requires_arg beside takes_arg (settings: those with a value to set), and the check refuses a bare one as wrong #args before any line runs; while a ctl runs, the first error a builtin reports fails the write, quoted with its line, and the prompt refusal quotes its line too. Docs say what a failure mid-write leaves done. Co-Authored-By: Claude Opus 5.5 --- src/Messages.zig | 6 ++++++ src/builtins.zig | 31 +++++++++++++++++++++++++++++++ src/fs.zig | 6 +++++- src/ninep/ctl.zig | 42 +++++++++++++++++++++++++++++++----------- 4 files changed, 73 insertions(+), 12 deletions(-) (limited to 'src') diff --git a/src/Messages.zig b/src/Messages.zig index c7ff09be..f4ea1c3e 100644 --- a/src/Messages.zig +++ b/src/Messages.zig @@ -388,6 +388,12 @@ pub fn messageLog(m: *const Messages, i: usize) ?*const LoggedMessage { pub fn reportError(p: *Pardes, id: usize, operation: []const u8, err: anyerror) void { var buf: [256]u8 = undefined; const text = std.fmt.bufPrint(&buf, "{s}: {s}", .{ operation, @errorName(err) }) catch operation; + // A builtin a ctl write runs: its first error is also the write's. + if (p.fs.no_prompt and p.fs.failure_len == 0) { + const n = @min(text.len, p.fs.failure.len); + @memcpy(p.fs.failure[0..n], text[0..n]); + p.fs.failure_len = @intCast(n); + } setMessage(p, id, text); } diff --git a/src/builtins.zig b/src/builtins.zig index ed0a07dd..2dac8a59 100644 --- a/src/builtins.zig +++ b/src/builtins.zig @@ -145,6 +145,22 @@ pub const registry = struct { return true; } + /// Whether the word means nothing without its argument (`Mount`, + /// `Msg`, a setting's value), or would ask for it at a prompt (`Find`): + /// a ctl refuses it bare before any line of the write runs. A builtin + /// says `pub const requires_arg = true;` beside `takes_arg`. + pub fn requiresArg(b: Builtin()) bool { + inline for (manualBuiltinList(), 0..) |T, i| + if (@intFromEnum(b) == i) return @hasDecl(T, "requires_arg") and T.requires_arg; + inline for (comptime settingList(), manualBuiltinCount()..) |setting, i| + if (@intFromEnum(b) == i) return switch (setting.action) { + // a switch flips bare, and DumpDir bare is the default + .toggle, .transition, .scene, .dump_dir => false, + .shell, .theme, .font, .tagline_size, .window_opacity, .window_blur, .message_ms => true, + }; + unreachable; + } + pub fn scope(b: Builtin()) Scope { inline for (manualBuiltinList(), 0..) |T, i| if (@intFromEnum(b) == i) return if (@hasDecl(T, "scope")) T.scope else .pane; @@ -237,6 +253,7 @@ comptime { pub const Look = struct { pub const takes_arg = true; + pub const requires_arg = true; // a look's answer is the pane it opens or the place it jumps to; its own // name on the message row would only be noise over that pub const quiet = true; @@ -247,6 +264,7 @@ pub const Look = struct { pub const Exec = struct { pub const takes_arg = true; + pub const requires_arg = true; pub fn run(c: Ctx) void { // the destination pane is Look's business (it focuses what answered); // an execute deliberately leaves you where you were @@ -311,6 +329,7 @@ pub const Detach = struct { pub const Mount = struct { pub const scope: Scope = .session; pub const takes_arg = true; + pub const requires_arg = true; pub const enabled = pardes.hosted; pub fn run(c: Ctx) void { if (comptime !enabled) unreachable; @@ -325,6 +344,7 @@ pub const Mount = struct { pub const Unmount = struct { pub const scope: Scope = .session; pub const takes_arg = true; + pub const requires_arg = true; pub const enabled = pardes.hosted; pub fn run(c: Ctx) void { if (comptime !enabled) unreachable; @@ -339,6 +359,7 @@ pub const Unmount = struct { pub const Msg = struct { pub const takes_arg = true; + pub const requires_arg = true; pub const quiet = true; // it IS the message row pub fn run(c: Ctx) void { if (c.arg) |text| @@ -366,6 +387,7 @@ pub const ThemeSel = struct { pub const ThemeFile = struct { pub const scope: Scope = .session; pub const takes_arg = true; + pub const requires_arg = true; pub const enabled = pardes.hosted; pub fn run(c: Ctx) void { if (comptime enabled) @@ -722,6 +744,7 @@ pub const Changelog = struct { pub const EffectCode = struct { pub const scope: Scope = .session; pub const takes_arg = true; + pub const requires_arg = true; pub const enabled = capabilities.panel_transitions or capabilities.scene_shaders; pub const output: OutputTraits = .{ .name = config.effect_code_buffer }; pub fn run(c: Ctx) void { @@ -737,6 +760,7 @@ pub const EffectCode = struct { pub const Pet = struct { pub const scope: Scope = .session; pub const takes_arg = true; + pub const requires_arg = true; pub const enabled = pardes.platform == .gui; pub fn run(c: Ctx) void { const name = std.mem.trim(u8, c.arg orelse return, " \t\r\n"); @@ -758,6 +782,7 @@ pub const Mini = struct { pub const Find = struct { pub const takes_arg = true; + pub const requires_arg = true; pub const output: OutputTraits = .{ .name = config.search_buffer, .steps = true }; pub fn run(c: Ctx) void { const pat = std.mem.trim(u8, c.arg orelse "", " \t\r\n"); @@ -771,6 +796,7 @@ pub const Find = struct { /// matches file CONTENTS under every pane's directory at once. pub const Grep = struct { pub const takes_arg = true; + pub const requires_arg = true; pub const output: OutputTraits = .{ .name = config.search_buffer, .steps = true, .locations = true }; pub fn run(c: Ctx) void { const pat = std.mem.trim(u8, c.arg orelse "", " \t\r\n"); @@ -940,6 +966,7 @@ pub const Subtypes = struct { pub const Rename = struct { pub const takes_arg = true; + pub const requires_arg = true; pub fn run(c: Ctx) void { const a = std.mem.trim(u8, c.arg orelse "", " \t\r\n"); if (a.len > 0) return c.p.lspRequest(c.id, .rename, a); @@ -949,6 +976,7 @@ pub const Rename = struct { pub const WsSymbols = struct { pub const takes_arg = true; + pub const requires_arg = true; pub fn run(c: Ctx) void { const a = std.mem.trim(u8, c.arg orelse "", " \t\r\n"); if (a.len > 0) return c.p.lspRequest(c.id, .workspace_symbols, a); @@ -971,6 +999,7 @@ pub const Lspwhy = struct { pub const Peek = struct { pub const scope: Scope = .session; pub const takes_arg = true; + pub const requires_arg = true; pub const enabled = Board.enabled; pub const output: OutputTraits = .{ .name = config.peek_buffer }; pub fn run(c: Ctx) void { @@ -984,6 +1013,7 @@ pub const Peek = struct { pub const Poke = struct { pub const scope: Scope = .session; pub const takes_arg = true; + pub const requires_arg = true; pub const enabled = Board.enabled; pub fn run(c: Ctx) void { if (comptime enabled) { @@ -996,6 +1026,7 @@ pub const Poke = struct { pub const Hexdump = struct { pub const scope: Scope = .session; pub const takes_arg = true; + pub const requires_arg = true; pub const enabled = Board.enabled; pub const output: OutputTraits = .{ .name = config.hexdump_buffer }; pub fn run(c: Ctx) void { diff --git a/src/fs.zig b/src/fs.zig index 339910a4..022a326a 100644 --- a/src/fs.zig +++ b/src/fs.zig @@ -1257,9 +1257,13 @@ pub const Namespace = struct { listeners: u16 = 0, origin: u8 = 'K', /// A ctl write is running builtins: one that would open a prompt for - /// its argument refuses (`refused`), since nobody is at the prompt. + /// its argument refuses (`refused`), since nobody is at the prompt, and + /// the first error one reports (`failure`) fails the write, as acme's + /// ctl answers a command's error (editors/acme/xfid.c:700). no_prompt: bool = false, refused: bool = false, + failure: [96]u8 = undefined, + failure_len: u8 = 0, /// A refusal that quotes the message it refuses, as Plan 9's cmderror /// does (kernel/misc/parse.c:82); answered at once (src/9p_io.zig). ename: [128]u8 = undefined, diff --git a/src/ninep/ctl.zig b/src/ninep/ctl.zig index 945119ca..e47dec16 100644 --- a/src/ninep/ctl.zig +++ b/src/ninep/ctl.zig @@ -180,7 +180,8 @@ fn refuse(p: *Pardes, req: Req, why: []const u8, line: []const u8) Reply { /// Checks one line written to a ctl as a builtin of `scope` before any line /// of the write runs: a word the registry knows, of this ctl's scope, given -/// an argument only if it takes one, and for a setting a value it takes. +/// an argument if and only if it takes or requires one, and for a setting a +/// value it takes. /// Answers the refusal, or null. Words are the builtins' own, capitalised /// as on a tag; acme's lowercase verbs are the pane ctl's and alias none. fn checkBuiltin(p: *Pardes, req: Req, line: []const u8, scope: builtins.Scope) ?Reply { @@ -191,6 +192,7 @@ fn checkBuiltin(p: *Pardes, req: Req, line: []const u8, scope: builtins.Scope) ? if (builtins.registry.scope(b) != scope) return refuse(p, req, if (scope == .pane) "not a window control message" else "not a session control message", line); if (arg.len > 0 and !builtins.registry.takesArg(b)) return refuse(p, req, "wrong #args in control message", line); + if (arg.len == 0 and builtins.registry.requiresArg(b)) return refuse(p, req, "wrong #args in control message", line); const setting = config.Runtime.find(word) orelse return null; const takes = switch (setting.action) { .theme => for (pardes.themes) |t| { @@ -206,14 +208,19 @@ fn checkBuiltin(p: *Pardes, req: Req, line: []const u8, scope: builtins.Scope) ? } /// Runs a checked control message at pane `id`, as a click on its word -/// would; false when it would have opened a prompt for an argument, which -/// nobody writing to a file is there to type. -fn runBuiltin(p: *Pardes, id: usize, line: []const u8) bool { +/// would. Answers the refusal when it would have opened a prompt for an +/// argument, which nobody writing to a file is there to type, or when it +/// reported an error: `Mount: AlreadyMounted "Mount peer /tmp/s"`. +fn runBuiltin(p: *Pardes, req: Req, id: usize, line: []const u8) ?Reply { p.fs.no_prompt = true; p.fs.refused = false; + p.fs.failure_len = 0; defer p.fs.no_prompt = false; _ = exec_line.executeBuiltinLine(p, id, line); - return !p.fs.refused; + if (p.fs.refused) return refuse(p, req, e_prompt, line); + if (p.fs.failure_len == 0) return null; + const refusal = refuse(p, req, p.fs.failure[0..p.fs.failure_len], line); + return .{ .tag = req.tag, .status = .err, .errno = E.IO, .ename = refusal.ename }; } const e_prompt = "control message needs its argument"; @@ -288,7 +295,7 @@ pub fn writeRoot(p: *Pardes, req: Req) Reply { continue; } if (p.panes[p.active] == null) return Reply.fail(req.tag, E.NOENT); - if (!runBuiltin(p, p.active, line)) return tree.failText(req.tag, E.INVAL, e_prompt); + if (runBuiltin(p, req, p.active, line)) |refusal| return refusal; } } return .{ .tag = req.tag, .written = @intCast(req.data.len) }; @@ -404,8 +411,8 @@ pub fn writePane(p: *Pardes, req: Req, pane: *Pane) Reply { } } else if (!apply) { if (checkBuiltin(p, req, line, .pane)) |refusal| return refusal; - } else if (!runBuiltin(p, p.paneBySerial(serial).?, line)) { - return tree.failText(req.tag, E.INVAL, e_prompt); + } else if (runBuiltin(p, req, p.paneBySerial(serial).?, line)) |refusal| { + return refusal; } } } @@ -542,13 +549,15 @@ test "the pane ctl takes acme's verbs and the pane's builtins, and refuses the r try testing.expectEqual(Status.ok, wr(p, ctl_node, "Msg from ctl\n").reply.status); const pane = p.panes[p.paneBySerial(serial).?].?; try testing.expectEqualStrings("from ctl", pane.msg[0..pane.msg_len]); - // One that would ask at a prompt for its argument fails instead. - try testing.expectEqualStrings(e_prompt, wr(p, ctl_node, "Find").reply.ename); + // One that means nothing bare is refused before anything runs. + try testing.expectEqualStrings("wrong #args in control message \"Find\"", wr(p, ctl_node, "Find").reply.ename); + try testing.expectEqual(E.INVAL, wr(p, ctl_node, "Msg first\nMsg").errno()); + try testing.expect(!std.mem.eql(u8, pane.msg[0..pane.msg_len], "first")); try testing.expect(pane.prompt == .none); try testing.expect(!p.fs.no_prompt); // Save with no name on a scratch would ask for one: refused, not asked. const other = try th.newPane(p); - try testing.expectEqualStrings(e_prompt, wr(p, Node.of(other, .ctl), "Save").reply.ename); + try testing.expectEqualStrings(e_prompt ++ " \"Save\"", wr(p, Node.of(other, .ctl), "Save").reply.ename); try testing.expect(p.panes[p.paneBySerial(other).?].?.prompt == .none); // A line after the one that closed the pane has nowhere to run. try testing.expectEqual(E.NOENT, wr(p, Node.of(other, .ctl), "Del\nMsg after").errno()); @@ -587,7 +596,18 @@ test "the root ctl reads the settings as a write takes them, and takes the sessi try testing.expectEqualStrings("bad value in control message \"Verbose maybe\"", wr(p, root_ctl, "Verbose maybe").reply.ename); try testing.expectEqualStrings("bad value in control message \"Theme no-such-theme\"", wr(p, root_ctl, "Theme no-such-theme").reply.ename); try testing.expectEqualStrings("wrong #args in control message \"Newcol 2\"", wr(p, root_ctl, "Newcol 2").reply.ename); + try testing.expectEqualStrings("wrong #args in control message \"Theme\"", wr(p, root_ctl, "Theme").reply.ename); + // A bare required word fails the check, so the line before never runs. + try testing.expectEqual(E.INVAL, wr(p, root_ctl, "Verbose on\nMount").errno()); try testing.expect(!p.settings.verbose); + // A builtin that fails as it runs fails the write, quoting its error and + // its line; the lines before it have taken effect, as in acme. + const failed = wr(p, root_ctl, "Verbose on\nMount x\nVerbose off"); + try testing.expectEqual(E.IO, failed.errno()); + try testing.expectEqualStrings("Mount name dial: MissingArgument \"Mount x\"", failed.reply.ename); + try testing.expect(p.settings.verbose); + try testing.expect(!p.fs.no_prompt); + try testing.expectEqual(Status.ok, wr(p, root_ctl, "Verbose off").reply.status); } -- cgit v1.3