A choice setting's refusal names what it takes, and /commands lists them

Bare Placement and BootShell flip, which is pardes's convention for a word
that chooses (Crt does too), but nothing said so, and "bad value in control
message" never said what would do. A refused value now says `takes acme,
pardes` (every switch and choice setting), /commands lists each such
setting's values in a fourth field, and config.md and fs.md say that bare
steps to the next. Motion and ShaderAnimation stepped nowhere bare and now
step like the rest.

Co-Authored-By: Claude Opus 5.5 <[email protected]>

3 files changed, 60 insertions, 21 deletions

diff --git a/src/builtins.zig b/src/builtins.zig
index 657f49d0..07cdf741 100644
--- a/src/builtins.zig
+++ b/src/builtins.zig
@@ -170,9 +170,9 @@ pub const registry = struct {
             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, .choice, .lift => false,
-                .shell, .theme, .font, .tagline_size, .window_opacity, .window_blur, .message_ms, .shader, .shader_animation, .motion, .inactive_dim, .grip_width => true,
+                // a switch flips bare, a choice steps, and DumpDir bare is the default
+                .toggle, .transition, .scene, .dump_dir, .choice, .lift, .shader_animation, .motion => false,
+                .shell, .theme, .font, .tagline_size, .window_opacity, .window_blur, .message_ms, .shader, .inactive_dim, .grip_width => true,
             };
         unreachable;
     }
diff --git a/src/config.zig b/src/config.zig
index 24c48770..234d8cae 100644
--- a/src/config.zig
+++ b/src/config.zig
@@ -1104,20 +1104,12 @@ pub const Runtime = struct {
                 }
                 return state.post.set(null, path, null);
             },
-            .shader_animation => {
-                const text = std.mem.trim(u8, argument orelse return false, " \t\r\n");
-                state.shader_animation = std.meta.stringToEnum(ShaderAnimation, text) orelse return false;
-            },
+            .shader_animation => state.shader_animation = stepped(ShaderAnimation, state.shader_animation, argument) orelse return false,
             // Bare, it steps to the next word, as a tag word should.
             .choice => |which| switch (which) {
                 inline else => |field| {
                     const now = &@field(state, @tagName(field));
-                    const T = @TypeOf(now.*);
-                    const text = std.mem.trim(u8, argument orelse "", " \t\r\n");
-                    now.* = if (text.len == 0)
-                        @enumFromInt((@as(usize, @intFromEnum(now.*)) + 1) % std.enums.values(T).len)
-                    else
-                        std.meta.stringToEnum(T, text) orelse return false;
+                    now.* = stepped(@TypeOf(now.*), now.*, argument) orelse return false;
                 },
             },
             .lift => {
@@ -1129,10 +1121,7 @@ pub const Runtime = struct {
                 else
                     std.meta.stringToEnum(LiftStyle, text) orelse return false;
             },
-            .motion => {
-                const text = std.mem.trim(u8, argument orelse return false, " \t\r\n");
-                state.motion = std.meta.stringToEnum(animation.Motion.Flavour, text) orelse return false;
-            },
+            .motion => state.motion = stepped(animation.Motion.Flavour, state.motion, argument) orelse return false,
             .inactive_dim => {
                 const text = std.mem.trim(u8, argument orelse return false, " \t\r\n%");
                 const percent = std.fmt.parseInt(u8, text, 10) catch return false;
@@ -1150,6 +1139,40 @@ pub const Runtime = struct {
         return true;
     }
 
+    /// The values a setting that chooses among words takes, for a refusal to
+    /// name; null for one that takes a number, a path or a name. Bare, each
+    /// of these flips (a two-valued one) or steps to its next value.
+    pub fn choices(action: Action) ?[]const u8 {
+        const words = struct {
+            fn of(comptime T: type) []const u8 {
+                comptime var out: []const u8 = "";
+                inline for (std.meta.fieldNames(T), 0..) |name, i| out = out ++ (if (i > 0) ", " else "") ++ name;
+                return out;
+            }
+        };
+        return switch (action) {
+            .toggle, .transition => "on, off",
+            .choice => |which| switch (which) {
+                inline else => |field| comptime words.of(@FieldType(Runtime, @tagName(field))),
+            },
+            .lift => "on, " ++ comptime words.of(LiftStyle),
+            .motion => comptime words.of(animation.Motion.Flavour),
+            .scene => std.fmt.comptimePrint("on, off, 0-{d}", .{Post.max_level}),
+            .shader_animation => comptime words.of(ShaderAnimation),
+            else => null,
+        };
+    }
+
+    /// The value a choice word names, or bare, the one after `now`; null for
+    /// a word it does not take.
+    fn stepped(comptime T: type, now: T, argument: ?[]const u8) ?T {
+        const text = std.mem.trim(u8, argument orelse "", " \t\r\n");
+        if (text.len > 0) return std.meta.stringToEnum(T, text);
+        const all = std.enums.values(T);
+        const at = std.mem.indexOfScalar(T, all, now) orelse 0;
+        return all[(at + 1) % all.len];
+    }
+
     /// What `on`, `off` or nothing after a switch word asks for: null flips
     /// it, as a bare tagline word does.
     fn switchedTo(argument: ?[]const u8) error{NotOnOrOff}!?bool {
diff --git a/src/ninep/ctl.zig b/src/ninep/ctl.zig
index c8be3bad..e62ff605 100644
--- a/src/ninep/ctl.zig
+++ b/src/ninep/ctl.zig
@@ -281,7 +281,12 @@ fn checkBuiltin(p: *Pardes, req: Req, line: []const u8, scope: builtins.Scope) ?
             break :probe probe.apply(setting, if (arg.len > 0) arg else null);
         },
     };
-    return if (takes) null else refuse(p, req, "bad value in control message", line);
+    if (takes) return null;
+    var why: [104]u8 = undefined;
+    return refuse(p, req, if (config.Runtime.choices(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);
 }
 
 /// Runs a checked control message at pane `id`, as a click on its word
@@ -407,11 +412,20 @@ pub fn writeRoot(p: *Pardes, req: Req) Reply {
 /// itself, so a new builtin is listed the moment it exists.
 fn writeCommands(w: *std.Io.Writer) !void {
     for (std.enums.values(Builtin)) |b| {
-        try w.print("{s}{s} {s}\n", .{
+        try w.print("{s}{s} {s}", .{
             @tagName(b),
             if (builtins.registry.takesArg(b)) " arg" else "",
             if (builtins.registry.scope(b) == .session) "root" else "pane",
         });
+        // A setting that chooses among words: the words, comma-joined. Bare,
+        // it steps to the next of them (a two-valued one flips).
+        if (config.Runtime.find(@tagName(b))) |setting| if (config.Runtime.choices(setting.action)) |values| {
+            try w.writeByte(' ');
+            var parts = std.mem.splitSequence(u8, values, ", ");
+            var first = true;
+            while (parts.next()) |v| : (first = false) try w.print("{s}{s}", .{ if (first) "" else ",", v });
+        };
+        try w.writeByte('\n');
     }
 }
 
@@ -623,10 +637,11 @@ test "commands lists every builtin with its argument and the ctl that takes it"
         const b = std.meta.stringToEnum(Builtin, words.next().?).?;
         if (builtins.registry.takesArg(b)) try testing.expectEqualStrings("arg", words.next().?);
         try testing.expectEqualStrings(if (builtins.registry.scope(b) == .session) "root" else "pane", words.next().?);
+        if (config.Runtime.find(@tagName(b))) |s| if (config.Runtime.choices(s.action) != null) try testing.expect(words.next() != null);
         try testing.expect(words.next() == null);
     }
     try testing.expectEqual(std.enums.values(Builtin).len, lines);
-    for ([_][]const u8{ "\nDel arg pane\n", "\nNewcol root\n", "\nSave arg pane\n", "\nVerbose arg root\n", "\nUndo pane\n", "\nRedo pane\n" }) |row|
+    for ([_][]const u8{ "\nDel arg pane\n", "\nNewcol root\n", "\nSave arg pane\n", "\nVerbose arg root on,off\n", "\nPlacement arg root acme,pardes\n", "\nUndo pane\n", "\nRedo pane\n" }) |row|
         try testing.expect(std.mem.indexOf(u8, listed.bytes, row) != null);
 }
 
@@ -752,7 +767,8 @@ 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/<n>/ctl", wr(p, root_ctl, "Del").reply.ename);
     try testing.expectEqualStrings("unknown control message \"Nonsense 1\"", wr(p, root_ctl, "Nonsense 1").reply.ename);
-    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; takes on, off \"Verbose maybe\"", wr(p, root_ctl, "Verbose maybe").reply.ename);
+    try testing.expectEqualStrings("bad value in control message; takes acme, pardes \"Placement east\"", wr(p, root_ctl, "Placement east").reply.ename);
     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, "; ThemeSel lists all \"Theme no-such-theme\""));