command: allow output on success, refactor

river/Control.zig

@@ -134,35 +134,40 @@ fn runCommand(
 
     const args = self.args_map.get(c.wl_resource_get_id(wl_resource)).?.value.items;
 
-    var failure_message: []const u8 = undefined;
-    command.run(util.gpa, seat, args, &failure_message) catch |err| {
-        if (err == command.Error.CommandFailed) {
-            defer util.gpa.free(failure_message);
-            const out = std.cstr.addNullByte(util.gpa, failure_message) catch {
-                c.zriver_command_callback_v1_send_failure(callback_resource, "out of memory");
+    var out: ?[]const u8 = null;
+    defer if (out) |s| util.gpa.free(s);
+    command.run(util.gpa, seat, args, &out) catch |err| {
+        const failure_message = switch (err) {
+            command.Error.NoCommand => "no command given",
+            command.Error.UnknownCommand => "unknown command",
+            command.Error.UnknownOption => "unknown option",
+            command.Error.NotEnoughArguments => "not enough arguments",
+            command.Error.TooManyArguments => "too many arguments",
+            command.Error.Overflow => "value out of bounds",
+            command.Error.InvalidCharacter => "invalid character in argument",
+            command.Error.InvalidDirection => "invalid direction. Must be 'next' or 'previous'",
+            command.Error.InvalidRgba => "invalid color format, must be #RRGGBB or #RRGGBBAA",
+            command.Error.OutOfMemory => {
+                c.wl_client_post_no_memory(wl_client);
                 return;
-            };
-            defer util.gpa.free(out);
-            c.zriver_command_callback_v1_send_failure(callback_resource, out);
-        } else {
-            c.zriver_command_callback_v1_send_failure(
-                callback_resource,
-                switch (err) {
-                    command.Error.NoCommand => "no command given",
-                    command.Error.UnknownCommand => "unknown command",
-                    command.Error.UnknownOption => "unknown option",
-                    command.Error.NotEnoughArguments => "not enough arguments",
-                    command.Error.TooManyArguments => "too many arguments",
-                    command.Error.Overflow => "value out of bounds",
-                    command.Error.InvalidCharacter => "invalid character in argument",
-                    command.Error.InvalidDirection => "invalid direction. Must be 'next' or 'previous'",
-                    command.Error.InvalidRgba => "invalid color format, must be #RRGGBB or #RRGGBBAA",
-                    command.Error.OutOfMemory => "out of memory",
-                    command.Error.CommandFailed => unreachable,
-                },
-            );
-        }
+            },
+            command.Error.Other => std.cstr.addNullByte(util.gpa, out.?) catch {
+                c.wl_client_post_no_memory(wl_client);
+                return;
+            },
+        };
+        defer if (err == command.Error.Other) util.gpa.free(failure_message);
+        c.zriver_command_callback_v1_send_failure(callback_resource, failure_message);
         return;
     };
-    c.zriver_command_callback_v1_send_success(callback_resource, "");
+
+    const success_message = if (out) |s|
+        std.cstr.addNullByte(util.gpa, s) catch {
+            c.wl_client_post_no_memory(wl_client);
+            return;
+        }
+    else
+        "";
+    defer if (out != null) util.gpa.free(success_message);
+    c.zriver_command_callback_v1_send_success(callback_resource, success_message);
 }

river/Seat.zig

@@ -275,11 +275,11 @@ pub fn handleMapping(self: *Self, keysym: c.xkb_keysym_t, modifiers: u32) bool {
     for (modes.items[self.mode_id].items) |mapping| {
         if (modifiers == mapping.modifiers and keysym == mapping.keysym) {
             // Execute the bound command
-            var failure_message: []const u8 = undefined;
+            var failure_message: ?[]const u8 = null;
             command.run(util.gpa, self, mapping.command_args, &failure_message) catch |err| {
                 // TODO: log the error
-                if (err == command.Error.CommandFailed)
-                    util.gpa.free(failure_message);
+                if (err == command.Error.Other)
+                    util.gpa.free(failure_message.?);
             };
             return true;
         }

river/command.zig

@@ -59,7 +59,7 @@ pub const Direction = enum {
 // zig fmt: off
 const str_to_impl_fn = [_]struct {
     name: []const u8,
-    impl: fn (*std.mem.Allocator, *Seat, []const []const u8, *[]const u8) Error!void,
+    impl: fn (*std.mem.Allocator, *Seat, []const []const u8, *?[]const u8) Error!void,
 }{
     .{ .name = "close",               .impl = impl.close },
     .{ .name = "declare-mode",        .impl = impl.declareMode },
@@ -94,27 +94,28 @@ pub const Error = error{
     InvalidRgba,
     UnknownOption,
     OutOfMemory,
-    CommandFailed,
+    Other,
 };
 
 /// Run a command for the given Seat. The `args` parameter is similar to the
 /// classic argv in that the command to be run is passed as the first argument.
-/// If the command fails with Error.CommandFailed, a failure message will be
-/// allocated and the slice pointed to by the `failure_message` parameter will
-/// be set to point to it. The caller is responsible for freeing this message
-/// in the case of failure.
+/// The optional slice passed as the out parameter must initially be set to
+/// null. If the command produces output or Error.Other is returned, the slice
+/// will be set to the output of the command or a failure message, respectively.
+/// The caller is then responsible for freeing that slice, which will be
+/// allocated using the provided allocator.
 pub fn run(
     allocator: *std.mem.Allocator,
     seat: *Seat,
     args: []const []const u8,
-    failure_message: *[]const u8,
+    out: *?[]const u8,
 ) Error!void {
+    std.debug.assert(out.* == null);
     if (args.len == 0) return Error.NoCommand;
 
-    const name = args[0];
     const impl_fn = for (str_to_impl_fn) |definition| {
-        if (std.mem.eql(u8, name, definition.name)) break definition.impl;
+        if (std.mem.eql(u8, args[0], definition.name)) break definition.impl;
     } else return Error.UnknownCommand;
 
-    try impl_fn(allocator, seat, args, failure_message);
+    try impl_fn(allocator, seat, args, out);
 }

river/command/close.zig

@@ -17,8 +17,6 @@
 
 const std = @import("std");
 
-const c = @import("../c.zig");
-
 const Error = @import("../command.zig").Error;
 const Seat = @import("../Seat.zig");
 
@@ -27,11 +25,9 @@ pub fn close(
     allocator: *std.mem.Allocator,
     seat: *Seat,
     args: []const []const u8,
-    failure_message: *[]const u8,
+    out: *?[]const u8,
 ) Error!void {
-    if (seat.focused_view) |view| {
-        // Note: we don't call arrange() here as it will be called
-        // automatically when the view is unmapped.
-        view.close();
-    }
+    // Note: we don't call arrange() here as it will be called
+    // automatically when the view is unmapped.
+    if (seat.focused_view) |view| view.close();
 }

river/command/declare_mode.zig

@@ -17,7 +17,6 @@
 
 const std = @import("std");
 
-const c = @import("../c.zig");
 const util = @import("../util.zig");
 
 const Error = @import("../command.zig").Error;
@@ -29,7 +28,7 @@ pub fn declareMode(
     allocator: *std.mem.Allocator,
     seat: *Seat,
     args: []const []const u8,
-    failure_message: *[]const u8,
+    out: *?[]const u8,
 ) Error!void {
     if (args.len < 2) return Error.NotEnoughArguments;
     if (args.len > 2) return Error.TooManyArguments;
@@ -38,17 +37,17 @@ pub fn declareMode(
     const new_mode_name = args[1];
 
     if (config.mode_to_id.get(new_mode_name) != null) {
-        failure_message.* = try std.fmt.allocPrint(
+        out.* = try std.fmt.allocPrint(
             allocator,
             "mode '{}' already exists and cannot be re-declared",
             .{new_mode_name},
         );
-        return Error.CommandFailed;
+        return Error.Other;
     }
 
     const owned_name = try std.mem.dupe(util.gpa, u8, new_mode_name);
     errdefer util.gpa.free(owned_name);
     try config.mode_to_id.putNoClobber(owned_name, config.modes.items.len);
     errdefer _ = config.mode_to_id.remove(owned_name);
-    try config.modes.append(std.ArrayList(Mapping).init(allocator));
+    try config.modes.append(std.ArrayList(Mapping).init(util.gpa));
 }

river/command/enter_mode.zig

@@ -25,7 +25,7 @@ pub fn enterMode(
     allocator: *std.mem.Allocator,
     seat: *Seat,
     args: []const []const u8,
-    failure_message: *[]const u8,
+    out: *?[]const u8,
 ) Error!void {
     if (args.len < 2) return Error.NotEnoughArguments;
     if (args.len > 2) return Error.TooManyArguments;
@@ -33,11 +33,11 @@ pub fn enterMode(
     const config = seat.input_manager.server.config;
     const target_mode = args[1];
     seat.mode_id = config.mode_to_id.getValue(target_mode) orelse {
-        failure_message.* = try std.fmt.allocPrint(
+        out.* = try std.fmt.allocPrint(
             allocator,
             "cannot enter non-existant mode '{}'",
             .{target_mode},
         );
-        return Error.CommandFailed;
+        return Error.Other;
     };
 }

river/command/exit.zig

@@ -27,7 +27,7 @@ pub fn exit(
     allocator: *std.mem.Allocator,
     seat: *Seat,
     args: []const []const u8,
-    failure_message: *[]const u8,
+    out: *?[]const u8,
 ) Error!void {
     if (args.len > 1) return Error.TooManyArguments;
     c.wl_display_terminate(seat.input_manager.server.wl_display);

river/command/focus_output.zig

@@ -17,10 +17,8 @@
 
 const std = @import("std");
 
-const c = @import("../c.zig");
-
-const Error = @import("../command.zig").Error;
 const Direction = @import("../command.zig").Direction;
+const Error = @import("../command.zig").Error;
 const Output = @import("../Output.zig");
 const Seat = @import("../Seat.zig");
 
@@ -30,13 +28,14 @@ pub fn focusOutput(
     allocator: *std.mem.Allocator,
     seat: *Seat,
     args: []const []const u8,
-    failure_message: *[]const u8,
+    out: *?[]const u8,
 ) Error!void {
     if (args.len < 2) return Error.NotEnoughArguments;
     if (args.len > 2) return Error.TooManyArguments;
 
     const direction = try Direction.parse(args[1]);
     const root = &seat.input_manager.server.root;
+
     // If the noop output is focused, there are no other outputs to switch to
     if (seat.focused_output == &root.noop_output) {
         std.debug.assert(root.outputs.len == 0);

river/command/focus_view.zig

@@ -17,10 +17,8 @@
 
 const std = @import("std");
 
-const c = @import("../c.zig");
-
-const Error = @import("../command.zig").Error;
 const Direction = @import("../command.zig").Direction;
+const Error = @import("../command.zig").Error;
 const Seat = @import("../Seat.zig");
 const View = @import("../View.zig");
 const ViewStack = @import("../view_stack.zig").ViewStack;
@@ -31,7 +29,7 @@ pub fn focusView(
     allocator: *std.mem.Allocator,
     seat: *Seat,
     args: []const []const u8,
-    failure_message: *[]const u8,
+    out: *?[]const u8,
 ) Error!void {
     if (args.len < 2) return Error.NotEnoughArguments;
     if (args.len > 2) return Error.TooManyArguments;

river/command/layout.zig

@@ -17,7 +17,7 @@
 
 const std = @import("std");
 
-const c = @import("../c.zig");
+const util = @import("../util.zig");
 
 const Error = @import("../command.zig").Error;
 const Seat = @import("../Seat.zig");
@@ -26,12 +26,12 @@ pub fn layout(
     allocator: *std.mem.Allocator,
     seat: *Seat,
     args: []const []const u8,
-    failure_message: *[]const u8,
+    out: *?[]const u8,
 ) Error!void {
     if (args.len < 2) return Error.NotEnoughArguments;
 
-    allocator.free(seat.focused_output.layout);
-    seat.focused_output.layout = try std.mem.join(allocator, " ", args[1..]);
+    util.gpa.free(seat.focused_output.layout);
+    seat.focused_output.layout = try std.mem.join(util.gpa, " ", args[1..]);
 
     seat.focused_output.arrangeViews();
     seat.input_manager.server.root.startTransaction();

river/command/map.zig

@@ -18,6 +18,7 @@
 const std = @import("std");
 
 const c = @import("../c.zig");
+const util = @import("../util.zig");
 
 const Error = @import("../command.zig").Error;
 const Mapping = @import("../Mapping.zig");
@@ -46,7 +47,7 @@ pub fn map(
     allocator: *std.mem.Allocator,
     seat: *Seat,
     args: []const []const u8,
-    failure_message: *[]const u8,
+    out: *?[]const u8,
 ) Error!void {
     if (args.len < 4) return Error.NotEnoughArguments;
 
@@ -54,12 +55,12 @@ pub fn map(
     const config = seat.input_manager.server.config;
     const target_mode = args[1];
     const mode_id = config.mode_to_id.getValue(target_mode) orelse {
-        failure_message.* = try std.fmt.allocPrint(
+        out.* = try std.fmt.allocPrint(
             allocator,
             "cannot add mapping to non-existant mode '{}p'",
             .{target_mode},
         );
-        return Error.CommandFailed;
+        return Error.Other;
     };
 
     // Parse the modifiers
@@ -72,12 +73,12 @@ pub fn map(
                 break;
             }
         } else {
-            failure_message.* = try std.fmt.allocPrint(
+            out.* = try std.fmt.allocPrint(
                 allocator,
                 "invalid modifier '{}'",
                 .{mod_name},
             );
-            return Error.CommandFailed;
+            return Error.Other;
         }
     }
 
@@ -86,26 +87,26 @@ pub fn map(
     defer allocator.free(keysym_name);
     const keysym = c.xkb_keysym_from_name(keysym_name, .XKB_KEYSYM_CASE_INSENSITIVE);
     if (keysym == c.XKB_KEY_NoSymbol) {
-        failure_message.* = try std.fmt.allocPrint(
+        out.* = try std.fmt.allocPrint(
             allocator,
             "invalid keysym '{}'",
             .{args[3]},
         );
-        return Error.CommandFailed;
+        return Error.Other;
     }
 
     // Check if the mapping already exists
     const mode_mappings = &config.modes.items[mode_id];
     for (mode_mappings.items) |existant_mapping| {
         if (existant_mapping.modifiers == modifiers and existant_mapping.keysym == keysym) {
-            failure_message.* = try std.fmt.allocPrint(
+            out.* = try std.fmt.allocPrint(
                 allocator,
                 "a mapping for modifiers '{}' and keysym '{}' already exists",
                 .{ args[2], args[3] },
             );
-            return Error.CommandFailed;
+            return Error.Other;
         }
     }
 
-    try mode_mappings.append(try Mapping.init(allocator, keysym, modifiers, args[4..]));
+    try mode_mappings.append(try Mapping.init(util.gpa, keysym, modifiers, args[4..]));
 }

river/command/mod_master_count.zig

@@ -17,8 +17,6 @@
 
 const std = @import("std");
 
-const c = @import("../c.zig");
-
 const Error = @import("../command.zig").Error;
 const Seat = @import("../Seat.zig");
 
@@ -27,16 +25,13 @@ pub fn modMasterCount(
     allocator: *std.mem.Allocator,
     seat: *Seat,
     args: []const []const u8,
-    failure_message: *[]const u8,
+    out: *?[]const u8,
 ) Error!void {
     if (args.len < 2) return Error.NotEnoughArguments;
     if (args.len > 2) return Error.TooManyArguments;
 
     const delta = try std.fmt.parseInt(i32, args[1], 10);
     const output = seat.focused_output;
-    output.master_count = @intCast(
-        u32,
-        std.math.max(0, @intCast(i32, output.master_count) + delta),
-    );
+    output.master_count = @intCast(u32, std.math.max(0, @intCast(i32, output.master_count) + delta));
     seat.input_manager.server.root.arrange();
 }

river/command/mod_master_factor.zig

@@ -17,8 +17,6 @@
 
 const std = @import("std");
 
-const c = @import("../c.zig");
-
 const Error = @import("../command.zig").Error;
 const Seat = @import("../Seat.zig");
 
@@ -27,17 +25,14 @@ pub fn modMasterFactor(
     allocator: *std.mem.Allocator,
     seat: *Seat,
     args: []const []const u8,
-    failure_message: *[]const u8,
+    out: *?[]const u8,
 ) Error!void {
     if (args.len < 2) return Error.NotEnoughArguments;
     if (args.len > 2) return Error.TooManyArguments;
 
     const delta = try std.fmt.parseFloat(f64, args[1]);
     const output = seat.focused_output;
-    const new_master_factor = std.math.min(
-        std.math.max(output.master_factor + delta, 0.05),
-        0.95,
-    );
+    const new_master_factor = std.math.min(std.math.max(output.master_factor + delta, 0.05), 0.95);
     if (new_master_factor != output.master_factor) {
         output.master_factor = new_master_factor;
         seat.input_manager.server.root.arrange();

river/command/send_to_output.zig

@@ -17,10 +17,8 @@
 
 const std = @import("std");
 
-const c = @import("../c.zig");
-
-const Error = @import("../command.zig").Error;
 const Direction = @import("../command.zig").Direction;
+const Error = @import("../command.zig").Error;
 const Output = @import("../Output.zig");
 const Seat = @import("../Seat.zig");
 
@@ -30,7 +28,7 @@ pub fn sendToOutput(
     allocator: *std.mem.Allocator,
     seat: *Seat,
     args: []const []const u8,
-    failure_message: *[]const u8,
+    out: *?[]const u8,
 ) Error!void {
     if (args.len < 2) return Error.NotEnoughArguments;
     if (args.len > 2) return Error.TooManyArguments;
@@ -45,7 +43,7 @@ pub fn sendToOutput(
             return;
         }
 
-        // Send to the next/preg output in the list if there is one, else wrap
+        // Send to the next/prev output in the list if there is one, else wrap
         const current_node = @fieldParentPtr(std.TailQueue(Output).Node, "data", view.output);
         const destination_output = switch (direction) {
             .Next => if (current_node.next) |node| &node.data else &root.outputs.first.?.data,

river/command/set_option.zig

@@ -35,13 +35,12 @@ pub fn setOption(
     allocator: *std.mem.Allocator,
     seat: *Seat,
     args: []const []const u8,
-    failure_message: *[]const u8,
+    out: *?[]const u8,
 ) Error!void {
     if (args.len < 3) return Error.NotEnoughArguments;
     if (args.len > 3) return Error.TooManyArguments;
 
     const config = &seat.input_manager.server.config;
-
     const option = std.meta.stringToEnum(Option, args[1]) orelse return Error.UnknownOption;
 
     // Assign value to option.
@@ -59,7 +58,7 @@ pub fn setOption(
 }
 
 /// Parse a color in the format #RRGGBB or #RRGGBBAA
-pub fn parseRgba(string: []const u8) ![4]f32 {
+fn parseRgba(string: []const u8) ![4]f32 {
     if (string[0] != '#' or (string.len != 7 and string.len != 9)) return error.InvalidRgba;
 
     const r = try std.fmt.parseInt(u8, string[1..3], 16);

river/command/spawn.zig

@@ -25,7 +25,7 @@ pub fn spawn(
     allocator: *std.mem.Allocator,
     seat: *Seat,
     args: []const []const u8,
-    failure_message: *[]const u8,
+    out: *?[]const u8,
 ) Error!void {
     if (args.len < 2) return Error.NotEnoughArguments;
 
@@ -37,11 +37,11 @@ pub fn spawn(
     defer child.deinit();
 
     std.ChildProcess.spawn(child) catch |err| {
-        failure_message.* = try std.fmt.allocPrint(
+        out.* = try std.fmt.allocPrint(
             allocator,
             "failed to spawn {}: {}.",
             .{ cmd, err },
         );
-        return Error.CommandFailed;
+        return Error.Other;
     };
 }

river/command/tags.zig

@@ -25,9 +25,9 @@ pub fn setFocusedTags(
     allocator: *std.mem.Allocator,
     seat: *Seat,
     args: []const []const u8,
-    failure_message: *[]const u8,
+    out: *?[]const u8,
 ) Error!void {
-    const tags = try parseTags(allocator, args, failure_message);
+    const tags = try parseTags(allocator, args, out);
     if (seat.focused_output.current_focused_tags != tags) {
         seat.focused_output.pending_focused_tags = tags;
         seat.input_manager.server.root.arrange();
@@ -39,9 +39,9 @@ pub fn setViewTags(
     allocator: *std.mem.Allocator,
     seat: *Seat,
     args: []const []const u8,
-    failure_message: *[]const u8,
+    out: *?[]const u8,
 ) Error!void {
-    const tags = try parseTags(allocator, args, failure_message);
+    const tags = try parseTags(allocator, args, out);
     if (seat.focused_view) |view| {
         if (view.current_tags != tags) {
             view.pending_tags = tags;
@@ -55,9 +55,9 @@ pub fn toggleFocusedTags(
     allocator: *std.mem.Allocator,
     seat: *Seat,
     args: []const []const u8,
-    failure_message: *[]const u8,
+    out: *?[]const u8,
 ) Error!void {
-    const tags = try parseTags(allocator, args, failure_message);
+    const tags = try parseTags(allocator, args, out);
     const output = seat.focused_output;
     const new_focused_tags = output.current_focused_tags ^ tags;
     if (new_focused_tags != 0) {
@@ -71,9 +71,9 @@ pub fn toggleViewTags(
     allocator: *std.mem.Allocator,
     seat: *Seat,
     args: []const []const u8,
-    failure_message: *[]const u8,
+    out: *?[]const u8,
 ) Error!void {
-    const tags = try parseTags(allocator, args, failure_message);
+    const tags = try parseTags(allocator, args, out);
     if (seat.focused_view) |view| {
         const new_tags = view.current_tags ^ tags;
         if (new_tags != 0) {
@@ -86,7 +86,7 @@ pub fn toggleViewTags(
 fn parseTags(
     allocator: *std.mem.Allocator,
     args: []const []const u8,
-    failure_message: *[]const u8,
+    out: *?[]const u8,
 ) Error!u32 {
     if (args.len < 2) return Error.NotEnoughArguments;
     if (args.len > 2) return Error.TooManyArguments;
@@ -94,8 +94,8 @@ fn parseTags(
     const tags = try std.fmt.parseInt(u32, args[1], 10);
 
     if (tags == 0) {
-        failure_message.* = try std.fmt.allocPrint(allocator, "tagmask may not be 0", .{});
-        return Error.CommandFailed;
+        out.* = try std.fmt.allocPrint(allocator, "tagmask may not be 0", .{});
+        return Error.Other;
     }
 
     return tags;

river/command/toggle_float.zig

@@ -17,8 +17,6 @@
 
 const std = @import("std");
 
-const c = @import("../c.zig");
-
 const Error = @import("../command.zig").Error;
 const Seat = @import("../Seat.zig");
 
@@ -28,7 +26,7 @@ pub fn toggleFloat(
     allocator: *std.mem.Allocator,
     seat: *Seat,
     args: []const []const u8,
-    failure_message: *[]const u8,
+    out: *?[]const u8,
 ) Error!void {
     if (args.len > 1) return Error.TooManyArguments;
     if (seat.focused_view) |view| {

river/command/zoom.zig

@@ -17,8 +17,6 @@
 
 const std = @import("std");
 
-const c = @import("../c.zig");
-
 const Error = @import("../command.zig").Error;
 const Seat = @import("../Seat.zig");
 const View = @import("../View.zig");
@@ -30,7 +28,7 @@ pub fn zoom(
     allocator: *std.mem.Allocator,
     seat: *Seat,
     args: []const []const u8,
-    failure_message: *[]const u8,
+    out: *?[]const u8,
 ) Error!void {
     if (args.len > 1) return Error.TooManyArguments;