From cdd67bcc577aa6f6d9194b412a6e0f9c91f0f619 Mon Sep 17 00:00:00 2001 From: Gabriel Schneider Date: Thu, 1 Oct 2026 01:34:47 -0300 Subject: A bad setting value is what the refusal quotes, so different bad values read and log as different records, never merged as one (x2) A setting's bad value quoted the setting's word, so "Verbose maybe" and "Verbose nope" made the same err text and the log's repeat count merged them. The value is the offending word, and is now the one quoted, blanks and all; the log's dedupe, comparing whole texts, then keeps them apart. Co-Authored-By: Claude Opus 5.5 --- src/ninep/ctl.zig | 39 ++++++++++++++++++++++++++++----------- 1 file changed, 28 insertions(+), 11 deletions(-) (limited to 'src') diff --git a/src/ninep/ctl.zig b/src/ninep/ctl.zig index 82001f32..a4979f7d 100644 --- a/src/ninep/ctl.zig +++ b/src/ninep/ctl.zig @@ -422,7 +422,11 @@ const Builtin = builtins.registry.Builtin(); /// Refuses a control message, quoting it the way Plan 9's cmderror does /// (kernel/misc/parse.c:82): `unknown control message "Bogus 3"`. fn refuse(p: *Pardes, req: Req, why: []const u8, line: []const u8) Reply { - const word = firstWord(line); + return refuseQuoting(p, req, why, firstWord(line)); +} + +/// `refuse` quoting `word` as given: a setting's bad value, blanks and all. +fn refuseQuoting(p: *Pardes, req: Req, why: []const u8, word: []const u8) Reply { // A word too long to quote whole gives up its middle (fitErr), so the // refusal keeps both its ends. var shown: [4096]u8 = undefined; @@ -507,7 +511,7 @@ fn checkBuiltin(p: *Pardes, req: Req, line: []const u8, scope: builtins.Scope) ? var near: [128]u8 = undefined; const room = @import("cloud9").fs.errmax -| (head.len + tail.len + line.len + 3); var why: [256]u8 = undefined; - return refuse(p, req, std.fmt.bufPrint(&why, head ++ "{s}" ++ tail, .{pardes.colors.themesNear(near[0..@min(room, near.len)], arg)}) catch "bad value in control message", line); + return refuse(p, req, std.fmt.bufPrint(&why, head ++ "{s}" ++ tail, .{pardes.colors.themesNear(near[0..@min(room, near.len)], arg)}) catch "bad value in control message", arg); }, .font => config.Runtime.FontSpec.parse(arg) != null, else => probe: { @@ -517,10 +521,12 @@ fn checkBuiltin(p: *Pardes, req: Req, line: []const u8, scope: builtins.Scope) ? }; if (takes) return null; var why: [160]u8 = undefined; - return refuse(p, req, if (config.Runtime.takes(setting.action)) |values| + // The offending word is the value: quoted, so two bad values read, + // and log, as two. + return refuseQuoting(p, req, if (config.Runtime.takes(setting.action)) |values| std.fmt.bufPrint(&why, "bad value in control message; takes {s}", .{values}) catch "bad value in control message" else - "bad value in control message", line); + "bad value in control message", arg); } /// Runs a checked control message at pane `id`, as a click on its word @@ -1295,16 +1301,16 @@ test "the root ctl reads the settings as a write takes them, and takes the sessi try testing.expectEqualStrings("not a session control message \"Del\": write it to pane//ctl", wr(p, root_ctl, "Del").reply.ename); try testing.expectEqualStrings("unknown control message \"Nonsense\"", wr(p, root_ctl, "Nonsense 1").reply.ename); - try testing.expectEqualStrings("bad value in control message; takes on, off \"Verbose\"", wr(p, root_ctl, "Verbose maybe").reply.ename); - try testing.expectEqualStrings("bad value in control message; takes acme, pardes \"Placement\"", wr(p, root_ctl, "Placement east").reply.ename); + try testing.expectEqualStrings("bad value in control message; takes on, off \"maybe\"", wr(p, root_ctl, "Verbose maybe").reply.ename); + try testing.expectEqualStrings("bad value in control message; takes acme, pardes \"east\"", wr(p, root_ctl, "Placement east").reply.ename); // A number's refusal names its range, a path's what it takes. - try testing.expectEqualStrings("bad value in control message; takes 0-100 (a percentage) \"InactiveDim\"", wr(p, root_ctl, "InactiveDim 200").reply.ename); - try testing.expectEqualStrings("bad value in control message; takes 0-60000 (milliseconds) \"MessageLinger\"", wr(p, root_ctl, "MessageLinger x").reply.ename); + try testing.expectEqualStrings("bad value in control message; takes 0-100 (a percentage) \"200\"", wr(p, root_ctl, "InactiveDim 200").reply.ename); + try testing.expectEqualStrings("bad value in control message; takes 0-60000 (milliseconds) \"x\"", wr(p, root_ctl, "MessageLinger x").reply.ename); for (config.Runtime.settings) |setting| if (setting.action != .theme) try testing.expect(config.Runtime.takes(setting.action) != null); const no_theme = wr(p, root_ctl, "Theme no-such-theme").reply.ename; try testing.expectStringStartsWith(no_theme, "bad value in control message; like it: n"); - try testing.expect(std.mem.endsWith(u8, no_theme, "; Themes lists all \"Theme\"")); + try testing.expect(std.mem.endsWith(u8, no_theme, "; Themes lists all \"no-such-theme\"")); try testing.expectEqualStrings("wrong #args in control message \"Newcol\"", 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. @@ -1832,7 +1838,18 @@ test "Pager is a setting of the root ctl: pardes by default, off taken, anything try testing.expectEqual(Status.ok, wr(p, root_ctl, "Pager off\n").reply.status); try testing.expectEqual(config.Runtime.Pager.off, p.settings.pager); try testing.expect(std.mem.indexOf(u8, rd(p, root_ctl, 0, 1 << 16).bytes, "Pager off\n") != null); - try testing.expectEqualStrings("bad value in control message; takes pardes, off \"Pager\"", wr(p, root_ctl, "Pager less\n").reply.ename); + try testing.expectEqualStrings("bad value in control message; takes pardes, off \"less\"", wr(p, root_ctl, "Pager less\n").reply.ename); +} + +test "a bad setting value is quoted, so two different bad values log as two records, never one counted twice" { + const p = try withFile(testing.allocator, "x\n"); + defer p.deinit(); + const root_ctl = @intFromEnum(tree.TopFile.ctl); + _ = wr(p, root_ctl, "Verbose maybe\n"); + _ = wr(p, root_ctl, "Verbose nope\n"); + try testing.expect(th.logHas(p, "takes on, off \"maybe\"\n")); + try testing.expect(th.logHas(p, "takes on, off \"nope\"\n")); + try testing.expect(!th.logHas(p, "(x2)")); } test "a bare :N or :N:M look addresses the pane itself, from its look, the root's and event write-back" { @@ -2064,7 +2081,7 @@ test "Joincol with no column to the right and Theme with no such theme say so" { try testing.expectEqual(E.INVAL, themed.errno()); try testing.expectStringStartsWith(themed.reply.ename, "bad value in control message; like it: "); try testing.expect(std.mem.indexOf(u8, themed.reply.ename, ": dr") != null); - try testing.expect(std.mem.endsWith(u8, themed.reply.ename, "\"Theme\"")); + try testing.expect(std.mem.endsWith(u8, themed.reply.ename, "\"drak\"")); // A click on the word says it on the message row. _ = wr(p, Node.of(serialOf(p), .exec), "Theme drak\n"); const pane = p.panes[p.active].?; -- cgit v1.3