map-pointer: minor fixes and cleanups

river/Cursor.zig

@@ -531,6 +531,10 @@ fn handlePointerMapping(self: *Self, event: *wlr.Pointer.event.Button, view: *Vi
                 .command => |args| {
                     self.seat.focus(view);
                     self.seat.runCommand(args);
+                    // This is mildly inefficient as running the command may have already
+                    // started a transaction. However we need to start one after the Seat.focus()
+                    // call in the case where it didn't.
+                    server.root.startTransaction();
                 },
             }
             break true;

river/Mode.zig

@@ -32,6 +32,7 @@ pub fn deinit(self: *Self) void {
     util.gpa.free(self.name);
     for (self.mappings.items) |m| m.deinit();
     self.mappings.deinit(util.gpa);
+    for (self.pointer_mappings.items) |*m| m.deinit();
     self.pointer_mappings.deinit(util.gpa);
     self.switch_mappings.deinit(util.gpa);
 }

river/PointerMapping.zig

@@ -17,17 +17,12 @@
 const Self = @This();
 
 const std = @import("std");
+const assert = std.debug.assert;
 const wlr = @import("wlroots");
 
 const util = @import("util.zig");
 
-pub const ActionType = enum {
+pub const Action = union(enum) {
-    move,
-    resize,
-    command,
-};
-
-pub const Action = union(ActionType) {
     move: void,
     resize: void,
     command: []const [:0]const u8,
@@ -36,31 +31,32 @@ pub const Action = union(ActionType) {
 event_code: u32,
 modifiers: wlr.Keyboard.ModifierMask,
 action: Action,
-arena: std.heap.ArenaAllocator,
+/// Owns the memory backing the arguments if action is a command.
+arena_state: std.heap.ArenaAllocator.State,
 
 pub fn init(
     event_code: u32,
     modifiers: wlr.Keyboard.ModifierMask,
-    action_type: ActionType,
+    action_type: std.meta.Tag(Action),
     command_args: []const [:0]const u8,
 ) !Self {
-    var arena: std.heap.ArenaAllocator = std.heap.ArenaAllocator.init(util.gpa);
+    assert(action_type == .command or command_args.len == 1);
+
+    var arena = std.heap.ArenaAllocator.init(util.gpa);
     errdefer arena.deinit();
 
     const action: Action = switch (action_type) {
-        ActionType.move => Action.move,
+        .move => .move,
-        ActionType.resize => Action.resize,
+        .resize => .resize,
-        ActionType.command => blk: {
+        .command => blk: {
-            const allocator: std.mem.Allocator = arena.allocator();
+            const arena_allocator = arena.allocator();
 
-            var owned_args = try std.ArrayListUnmanaged([:0]const u8).initCapacity(allocator, command_args.len);
+            const owned_args = try arena_allocator.alloc([:0]const u8, command_args.len);
-
+            for (command_args) |arg, i| {
-            for (command_args) |arg| {
+                owned_args[i] = try arena_allocator.dupeZ(u8, arg);
-                const owned = try allocator.dupeZ(u8, arg);
-                owned_args.appendAssumeCapacity(owned);
             }
 
-            break :blk Action{ .command = owned_args.toOwnedSlice(allocator) };
+            break :blk .{ .command = owned_args };
         },
     };
 
@@ -68,10 +64,11 @@ pub fn init(
         .event_code = event_code,
         .modifiers = modifiers,
         .action = action,
-        .arena = arena,
+        .arena_state = arena.state,
     };
 }
 
 pub fn deinit(self: *Self) void {
-    self.arena.deinit();
+    self.arena_state.promote(util.gpa).deinit();
+    self.* = undefined;
 }

river/Seat.zig

@@ -442,12 +442,7 @@ pub fn runCommand(self: *Self, args: []const [:0]const u8) void {
             command.Error.Other => out.?,
             else => command.errToMsg(err),
         };
-
+        std.log.scoped(.command).err("{s}: {s}", .{ args[0], failure_message });
-        if (args.len == 0) {
-            std.log.scoped(.command).err("{s}", .{failure_message});
-        } else {
-            std.log.scoped(.command).err("{s}: {s}", .{ args[0], failure_message });
-        }
         return;
     };
     if (out) |s| {

river/command/map.zig

@@ -17,6 +17,7 @@
 const std = @import("std");
 const fmt = std.fmt;
 const mem = std.mem;
+const meta = std.meta;
 const wlr = @import("wlroots");
 const xkb = @import("xkbcommon");
 
@@ -135,14 +136,17 @@ pub fn mapPointer(
     const modifiers = try parseModifiers(args[2], out);
     const event_code = try parseEventCode(args[3], out);
 
-    const action = if (mem.eql(u8, args[4], "move-view"))
+    const action: meta.Tag(PointerMapping.Action) = blk: {
-        PointerMapping.ActionType.move
+        if (mem.eql(u8, args[4], "move-view")) {
-    else if (mem.eql(u8, args[4], "resize-view"))
+            break :blk .move;
-        PointerMapping.ActionType.resize
+        } else if (mem.eql(u8, args[4], "resize-view")) {
-    else
+            break :blk .resize;
-        PointerMapping.ActionType.command;
+        } else {
+            break :blk .command;
+        }
+    };
 
-    if (action != PointerMapping.ActionType.command and args.len > 5) return Error.TooManyArguments;
+    if (action != .command and args.len > 5) return Error.TooManyArguments;
 
     var new = try PointerMapping.init(
         event_code,
@@ -181,7 +185,7 @@ fn mappingExists(
     release: bool,
 ) ?usize {
     for (mappings.items) |mapping, i| {
-        if (std.meta.eql(mapping.modifiers, modifiers) and mapping.keysym == keysym and mapping.release == release) {
+        if (meta.eql(mapping.modifiers, modifiers) and mapping.keysym == keysym and mapping.release == release) {
             return i;
         }
     }
@@ -196,7 +200,7 @@ fn switchMappingExists(
     switch_state: Switch.State,
 ) ?usize {
     for (switch_mappings.items) |mapping, i| {
-        if (mapping.switch_type == switch_type and std.meta.eql(mapping.switch_state, switch_state)) {
+        if (mapping.switch_type == switch_type and meta.eql(mapping.switch_state, switch_state)) {
             return i;
         }
     }
@@ -211,7 +215,7 @@ fn pointerMappingExists(
     event_code: u32,
 ) ?usize {
     for (pointer_mappings.items) |mapping, i| {
-        if (std.meta.eql(mapping.modifiers, modifiers) and mapping.event_code == event_code) {
+        if (meta.eql(mapping.modifiers, modifiers) and mapping.event_code == event_code) {
             return i;
         }
     }
@@ -270,7 +274,7 @@ fn parseSwitchType(
     switch_type_str: []const u8,
     out: *?[]const u8,
 ) !Switch.Type {
-    return std.meta.stringToEnum(Switch.Type, switch_type_str) orelse {
+    return meta.stringToEnum(Switch.Type, switch_type_str) orelse {
         out.* = try std.fmt.allocPrint(
             util.gpa,
             "invalid switch '{s}', must be 'lid' or 'tablet'",
@@ -287,7 +291,7 @@ fn parseSwitchState(
 ) !Switch.State {
     switch (switch_type) {
         .lid => {
-            const lid_state = std.meta.stringToEnum(
+            const lid_state = meta.stringToEnum(
                 Switch.LidState,
                 switch_state_str,
             ) orelse {
@@ -301,7 +305,7 @@ fn parseSwitchState(
             return Switch.State{ .lid = lid_state };
         },
         .tablet => {
-            const tablet_state = std.meta.stringToEnum(
+            const tablet_state = meta.stringToEnum(
                 Switch.TabletState,
                 switch_state_str,
             ) orelse {