river: refactor keyboard groups implementation

This reduces the impact of keyboard groups on the Keyboard.zig
implementation and otherwise improves consistency with patterns used
elsewhere in rivers code.

There are also two small changes to the riverctl interface:

- keyboard-group-add-keyboard is renamed to keyboard-group-add
- keyboard-group-remove is added to support removing keyboards from a
group.

completions/bash/riverctl

@@ -5,7 +5,8 @@ function __riverctl_completion ()
 		OPTS=" \
 			keyboard-group-create \
 			keyboard-group-destroy \
-			keyboard-group-add-keyboard \
+			keyboard-group-add \
+			keyboard-group-remove \
 			csd-filter-add \
 			exit \
 			float-filter-add \

completions/fish/riverctl.fish

@@ -62,9 +62,10 @@ complete -c riverctl -x -n '__fish_riverctl_complete_arg 1' -a 'set-repeat'
 complete -c riverctl -x -n '__fish_riverctl_complete_arg 1' -a 'set-cursor-warp'        -d 'Set the cursor warp mode.'
 complete -c riverctl -x -n '__fish_riverctl_complete_arg 1' -a 'xcursor-theme'          -d 'Set the xcursor theme'
 # Keyboardgroups
-complete -c riverctl -x -n '__fish_riverctl_complete_arg 1' -a 'keyboard-group-create'       -d 'Create a keyboard group.'
+complete -c riverctl -x -n '__fish_riverctl_complete_arg 1' -a 'keyboard-group-create'  -d 'Create a keyboard group.'
-complete -c riverctl -x -n '__fish_riverctl_complete_arg 1' -a 'keyboard-group-destroy'      -d 'Destroy a keyboard group.'
+complete -c riverctl -x -n '__fish_riverctl_complete_arg 1' -a 'keyboard-group-destroy' -d 'Destroy a keyboard group.'
-complete -c riverctl -x -n '__fish_riverctl_complete_arg 1' -a 'keyboard-group-add-keyboard' -d 'Add a keyboard to a keyboard group.'
+complete -c riverctl -x -n '__fish_riverctl_complete_arg 1' -a 'keyboard-group-add'     -d 'Add a keyboard to a keyboard group.'
+complete -c riverctl -x -n '__fish_riverctl_complete_arg 1' -a 'keyboard-group-remove'  -d 'Remove a keyboard from a keyboard group.'
 
 # Subcommands
 complete -c riverctl -x -n '__fish_seen_subcommand_from focus-output'         -a 'next previous'

completions/zsh/_riverctl

@@ -58,7 +58,8 @@ _riverctl_subcommands()
         # Keyboard groups
         'keyboard-group-create:Create a keyboard group'
         'keyboard-group-destroy:Destroy a keyboard group'
-        'keyboard-group-add-keyboard:Add a keyboard to a keyboard group'
+        'keyboard-group-add:Add a keyboard to a keyboard group'
+        'keyboard-group-remove:Remove a keyboard from a keyboard group'
         # Input
         'input:Configure input devices'
         'list-inputs:List all input devices'

doc/riverctl.1.scd

@@ -330,26 +330,30 @@ A complete list may be found in _/usr/include/linux/input-event-codes.h_
 *list-input-configs*
 	List all input configurations.
 
-*keyboard-group-create* _keyboard_group_name_
+*keyboard-group-create* _group_name_
 	Create a keyboard group. A keyboard group collects multiple keyboards in
 	a single logical keyboard. This means that all state, like the active
 	modifiers, is shared between the keyboards in a group.
 
-*keyboard-group-destroy* _keyboard_group_name_
+*keyboard-group-destroy* _group_name_
-	Destroy the keyboard group of the given name. All attached keyboards
+	Destroy the keyboard group with the given name. All attached keyboards
 	will be released, making them act as seperate devices again.
 
-*keyboard-group-add-keyboard* _keyboard_group_name_ _input_device_identifier_
+*keyboard-group-add* _group_name_ _input_device_name_
-	Add a keyboard to a keyboard group, identified by the keyboards input
+	Add a keyboard to a keyboard group, identified by the keyboard's
-	device identifier. Any currently connected and future keyboards matching
+	input device name. Any currently connected and future keyboards with
-	the identifier will be added to the group.
+	the given name will be added to the group.
+
+*keyboard-group-remove* _group_name_ _input_device_name_
+	Remove a keyboard from a keyboard group, identified by the keyboard's
+	input device name.
 
 The _input_ command can be used to create a configuration rule for an input
 device identified by its _name_.
 The _name_ of an input device consists of its type, its numerical vendor id,
 its numerical product id and finally its self-advertised name, separated by -.
 
-A list of all device properties that can be configured maybe found below.
+A list of all device properties that can be configured may be found below.
 However note that not every input device supports every property.
 
 *input* _name_ *events* *enabled*|*disabled*|*disabled-on-external-mouse*

river/InputDevice.zig

@@ -50,6 +50,12 @@ pub fn init(device: *InputDevice, seat: *Seat, wlr_device: *wlr.InputDevice) !vo
         else => @tagName(wlr_device.type),
     };
 
+    // wlroots 0.15 leaves wlr_input_device->name NULL for keyboard groups.
+    // This wart has been cleaned up in 0.16, so just work around it until that is released.
+    // TODO(wlroots): Remove this hack
+
+    const name = if (isKeyboardGroup(wlr_device)) "wlr_keyboard_group" else wlr_device.name;
+
     const identifier = try std.fmt.allocPrint(
         util.gpa,
         "{s}-{}-{}-{s}",
@@ -57,7 +63,7 @@ pub fn init(device: *InputDevice, seat: *Seat, wlr_device: *wlr.InputDevice) !vo
             device_type,
             wlr_device.vendor,
             wlr_device.product,
-            mem.trim(u8, mem.span(wlr_device.name), &ascii.spaces),
+            mem.trim(u8, mem.span(name), &ascii.spaces),
         },
     );
     errdefer util.gpa.free(identifier);
@@ -77,15 +83,19 @@ pub fn init(device: *InputDevice, seat: *Seat, wlr_device: *wlr.InputDevice) !vo
 
     wlr_device.events.destroy.add(&device.destroy);
 
-    // Apply any matching input device configuration.
+    // Keyboard groups are implemented as "virtual" input devices which we don't want to expose
-    for (server.input_manager.configs.items) |*input_config| {
+    // in riverctl list-inputs as they can't be configured.
-        if (mem.eql(u8, input_config.identifier, identifier)) {
+    if (!isKeyboardGroup(wlr_device)) {
-            input_config.apply(device);
+        // Apply any matching input device configuration.
+        for (server.input_manager.configs.items) |*input_config| {
+            if (mem.eql(u8, input_config.identifier, identifier)) {
+                input_config.apply(device);
+            }
         }
-    }
 
-    server.input_manager.devices.append(device);
+        server.input_manager.devices.append(device);
-    seat.updateCapabilities();
+        seat.updateCapabilities();
+    }
 
     log.debug("new input device: {s}", .{identifier});
 }
@@ -95,12 +105,19 @@ pub fn deinit(device: *InputDevice) void {
 
     util.gpa.free(device.identifier);
 
-    device.link.remove();
+    if (!isKeyboardGroup(device.wlr_device)) {
-    device.seat.updateCapabilities();
+        device.link.remove();
+        device.seat.updateCapabilities();
+    }
 
     device.* = undefined;
 }
 
+fn isKeyboardGroup(wlr_device: *wlr.InputDevice) bool {
+    return wlr_device.type == .keyboard and
+        wlr.KeyboardGroup.fromKeyboard(wlr_device.device.keyboard) != null;
+}
+
 fn handleDestroy(listener: *wl.Listener(*wlr.InputDevice), _: *wlr.InputDevice) void {
     const device = @fieldParentPtr(InputDevice, "destroy", listener);
 
@@ -108,7 +125,7 @@ fn handleDestroy(listener: *wl.Listener(*wlr.InputDevice), _: *wlr.InputDevice)
 
     switch (device.wlr_device.type) {
         .keyboard => {
-            const keyboard = @fieldParentPtr(Keyboard, "provider", @ptrCast(*Keyboard.Provider, device));
+            const keyboard = @fieldParentPtr(Keyboard, "device", device);
             keyboard.deinit();
             util.gpa.destroy(keyboard);
         },

river/Keyboard.zig

@@ -25,19 +25,13 @@ const xkb = @import("xkbcommon");
 const server = &@import("main.zig").server;
 const util = @import("util.zig");
 
+const KeycodeSet = @import("KeycodeSet.zig");
 const Seat = @import("Seat.zig");
 const InputDevice = @import("InputDevice.zig");
-const KeyboardGroup = @import("KeyboardGroup.zig");
-const KeycodeSet = @import("KeycodeSet.zig");
 
 const log = std.log.scoped(.keyboard);
 
-pub const Provider = union(enum) {
+device: InputDevice,
-    device: InputDevice,
-    group: *KeyboardGroup,
-};
-
-provider: Provider,
 
 /// Pressed keys for which a mapping was triggered on press
 eaten_keycodes: KeycodeSet = .{},
@@ -46,8 +40,11 @@ key: wl.Listener(*wlr.Keyboard.event.Key) = wl.Listener(*wlr.Keyboard.event.Key)
 modifiers: wl.Listener(*wlr.Keyboard) = wl.Listener(*wlr.Keyboard).init(handleModifiers),
 
 pub fn init(self: *Self, seat: *Seat, wlr_device: *wlr.InputDevice) !void {
-    const wlr_keyboard = wlr_device.device.keyboard;
+    self.* = .{
-    wlr_keyboard.data = @ptrToInt(self);
+        .device = undefined,
+    };
+    try self.device.init(seat, wlr_device);
+    errdefer self.device.deinit();
 
     const context = xkb.Context.new(.no_flags) orelse return error.XkbContextFailed;
     defer context.unref();
@@ -57,70 +54,37 @@ pub fn init(self: *Self, seat: *Seat, wlr_device: *wlr.InputDevice) !void {
     const keymap = xkb.Keymap.newFromNames(context, null, .no_flags) orelse return error.XkbKeymapFailed;
     defer keymap.unref();
 
+    const wlr_keyboard = self.device.wlr_device.device.keyboard;
+    wlr_keyboard.data = @ptrToInt(self);
+
     if (!wlr_keyboard.setKeymap(keymap)) return error.SetKeymapFailed;
 
-    self.* = .{
+    wlr_keyboard.setRepeatInfo(server.config.repeat_rate, server.config.repeat_delay);
-        .provider = undefined,
-    };
-    if (wlr.KeyboardGroup.fromKeyboard(wlr_keyboard)) |grp| {
-        const group = @intToPtr(*KeyboardGroup, grp.data);
-        self.provider = .{ .group = group };
-    } else {
-        self.provider = .{ .device = undefined };
-        try self.provider.device.init(seat, wlr_device);
-    }
 
     wlr_keyboard.events.key.add(&self.key);
     wlr_keyboard.events.modifiers.add(&self.modifiers);
-
-    wlr_keyboard.setRepeatInfo(server.config.repeat_rate, server.config.repeat_delay);
 }
 
 pub fn deinit(self: *Self) void {
     self.key.link.remove();
     self.modifiers.link.remove();
 
-    switch (self.provider) {
+    self.device.deinit();
-        .device => {
-            const wlr_keyboard = self.provider.device.wlr_device.device.keyboard;
-            if (wlr_keyboard.group) |group| group.removeKeyboard(wlr_keyboard);
-            self.provider.device.deinit();
-        },
-        .group => {},
-    }
 
     self.* = undefined;
 }
 
-/// This event is raised when a key is pressed or released.
 fn handleKey(listener: *wl.Listener(*wlr.Keyboard.event.Key), event: *wlr.Keyboard.event.Key) void {
+    // This event is raised when a key is pressed or released.
     const self = @fieldParentPtr(Self, "key", listener);
-    switch (self.provider) {
+    const wlr_keyboard = self.device.wlr_device.device.keyboard;
-        .device => {
-            if (self.provider.device.wlr_device.device.keyboard.group != null) return;
-            self.handleKeyImpl(self.provider.device.seat, self.provider.device.wlr_device, event);
-        },
-        .group => self.handleKeyImpl(self.provider.group.seat, self.provider.group.group.input_device, event),
-    }
-}
 
-/// Simply pass modifiers along to the client
+    // If the keyboard is in a group, this event will be handled by the group's Keyboard instance.
-fn handleModifiers(listener: *wl.Listener(*wlr.Keyboard), _: *wlr.Keyboard) void {
+    if (wlr_keyboard.group != null) return;
-    const self = @fieldParentPtr(Self, "modifiers", listener);
-    switch (self.provider) {
-        .device => {
-            if (self.provider.device.wlr_device.device.keyboard.group != null) return;
-            self.handleModifiersImpl(self.provider.device.seat, self.provider.device.wlr_device);
-        },
-        .group => self.handleModifiersImpl(self.provider.group.seat, self.provider.group.group.input_device),
-    }
-}
 
-fn handleKeyImpl(self: *Self, seat: *Seat, wlr_device: *wlr.InputDevice, event: *wlr.Keyboard.event.Key) void {
+    self.device.seat.handleActivity();
-    const wlr_keyboard = wlr_device.device.keyboard;
 
-    seat.handleActivity();
+    self.device.seat.clearRepeatingMapping();
-    seat.clearRepeatingMapping();
 
     // Translate libinput keycode -> xkbcommon
     const keycode = event.keycode + 8;
@@ -137,7 +101,7 @@ fn handleKeyImpl(self: *Self, seat: *Seat, wlr_device: *wlr.InputDevice, event:
             !released and
             !isModifier(sym))
         {
-            seat.cursor.hide();
+            self.device.seat.cursor.hide();
             break;
         }
     }
@@ -148,11 +112,11 @@ fn handleKeyImpl(self: *Self, seat: *Seat, wlr_device: *wlr.InputDevice, event:
     }
 
     // Handle user-defined mappings
-    const mapped = seat.hasMapping(keycode, modifiers, released, xkb_state);
+    const mapped = self.device.seat.hasMapping(keycode, modifiers, released, xkb_state);
     if (mapped) {
         if (!released) self.eaten_keycodes.add(event.keycode);
 
-        const handled = seat.handleMapping(keycode, modifiers, released, xkb_state);
+        const handled = self.device.seat.handleMapping(keycode, modifiers, released, xkb_state);
         assert(handled);
     }
 
@@ -160,8 +124,8 @@ fn handleKeyImpl(self: *Self, seat: *Seat, wlr_device: *wlr.InputDevice, event:
 
     if (!eaten) {
         // If key was not handled, we pass it along to the client.
-        const wlr_seat = seat.wlr_seat;
+        const wlr_seat = self.device.seat.wlr_seat;
-        wlr_seat.setKeyboard(wlr_device);
+        wlr_seat.setKeyboard(self.device.wlr_device);
         wlr_seat.keyboardNotifyKey(event.time_msec, event.keycode, event.state);
     }
 }
@@ -170,6 +134,17 @@ fn isModifier(keysym: xkb.Keysym) bool {
     return @enumToInt(keysym) >= xkb.Keysym.Shift_L and @enumToInt(keysym) <= xkb.Keysym.Hyper_R;
 }
 
+fn handleModifiers(listener: *wl.Listener(*wlr.Keyboard), _: *wlr.Keyboard) void {
+    const self = @fieldParentPtr(Self, "modifiers", listener);
+    const wlr_keyboard = self.device.wlr_device.device.keyboard;
+
+    // If the keyboard is in a group, this event will be handled by the group's Keyboard instance.
+    if (wlr_keyboard.group != null) return;
+
+    self.device.seat.wlr_seat.setKeyboard(self.device.wlr_device);
+    self.device.seat.wlr_seat.keyboardNotifyModifiers(&wlr_keyboard.modifiers);
+}
+
 /// Handle any builtin, harcoded compsitor mappings such as VT switching.
 /// Returns true if the keysym was handled.
 fn handleBuiltinMapping(keysym: xkb.Keysym) bool {
@@ -189,9 +164,3 @@ fn handleBuiltinMapping(keysym: xkb.Keysym) bool {
         else => return false,
     }
 }
-
-/// Simply pass modifiers along to the client
-fn handleModifiersImpl(_: *Self, seat: *Seat, wlr_device: *wlr.InputDevice) void {
-    seat.wlr_seat.setKeyboard(wlr_device);
-    seat.wlr_seat.keyboardNotifyModifiers(&wlr_device.device.keyboard.modifiers);
-}

river/KeyboardGroup.zig

@@ -14,12 +14,12 @@
 // You should have received a copy of the GNU General Public License
 // along with this program. If not, see <https://www.gnu.org/licenses/>.
 
-const Self = @This();
+const KeyboardGroup = @This();
 
 const std = @import("std");
-const heap = std.heap;
+const assert = std.debug.assert;
 const mem = std.mem;
-const debug = std.debug;
+
 const wlr = @import("wlroots");
 const wl = @import("wayland").server.wl;
 const xkb = @import("xkbcommon");
@@ -33,81 +33,95 @@ const Seat = @import("Seat.zig");
 const Keyboard = @import("Keyboard.zig");
 
 seat: *Seat,
-group: *wlr.KeyboardGroup,
+wlr_group: *wlr.KeyboardGroup,
 name: []const u8,
-keyboard_identifiers: std.ArrayListUnmanaged([]const u8) = .{},
+identifiers: std.StringHashMapUnmanaged(void) = .{},
 
-pub fn init(self: *Self, seat: *Seat, _name: []const u8) !void {
+pub fn create(seat: *Seat, name: []const u8) !void {
-    log.debug("new keyboard group: '{s}'", .{_name});
+    log.debug("new keyboard group: '{s}'", .{name});
 
-    const group = try wlr.KeyboardGroup.create();
+    const node = try util.gpa.create(std.TailQueue(KeyboardGroup).Node);
-    errdefer group.destroy();
+    errdefer util.gpa.destroy(node);
-    group.data = @ptrToInt(self);
 
-    const name = try util.gpa.dupe(u8, _name);
+    const wlr_group = try wlr.KeyboardGroup.create();
-    errdefer util.gpa.free(name);
+    errdefer wlr_group.destroy();
 
-    self.* = .{
+    const owned_name = try util.gpa.dupe(u8, name);
-        .group = group,
+    errdefer util.gpa.free(owned_name);
-        .name = name,
+
+    node.data = .{
+        .wlr_group = wlr_group,
+        .name = owned_name,
         .seat = seat,
     };
 
-    seat.addDevice(self.group.input_device);
+    seat.addDevice(wlr_group.input_device);
-    seat.wlr_seat.setKeyboard(self.group.input_device);
+    seat.keyboard_groups.append(node);
 }
 
-pub fn deinit(self: *Self) void {
+pub fn destroy(group: *KeyboardGroup) void {
-    log.debug("removing keyboard group: '{s}'", .{self.name});
+    log.debug("destroying keyboard group: '{s}'", .{group.name});
 
-    util.gpa.free(self.name);
+    util.gpa.free(group.name);
-    for (self.keyboard_identifiers.items) |id| util.gpa.free(id);
+    {
-    self.keyboard_identifiers.deinit(util.gpa);
+        var it = group.identifiers.keyIterator();
+        while (it.next()) |id| util.gpa.free(id.*);
+    }
+    group.identifiers.deinit(util.gpa);
 
-    // wlroots automatically removes all keyboards from the group.
+    group.wlr_group.destroy();
-    self.group.destroy();
+
+    const node = @fieldParentPtr(std.TailQueue(KeyboardGroup).Node, "data", group);
+    group.seat.keyboard_groups.remove(node);
+    util.gpa.destroy(node);
 }
 
-pub fn addKeyboardIdentifier(self: *Self, _id: []const u8) !void {
+pub fn addIdentifier(group: *KeyboardGroup, new_id: []const u8) !void {
-    if (containsIdentifier(self, _id)) return;
+    if (group.identifiers.contains(new_id)) return;
-    log.debug("keyboard group '{s}' adding identifier: '{s}'", .{ self.name, _id });
 
-    const id = try util.gpa.dupe(u8, _id);
+    log.debug("keyboard group '{s}' adding identifier: '{s}'", .{ group.name, new_id });
-    errdefer util.gpa.free(id);
-    try self.keyboard_identifiers.append(util.gpa, id);
 
-    // Add any existing matching keyboard to group.
+    const owned_id = try util.gpa.dupe(u8, new_id);
+    errdefer util.gpa.free(owned_id);
+
+    try group.identifiers.put(util.gpa, owned_id, {});
+
+    // Add any existing matching keyboards to the group.
     var it = server.input_manager.devices.iterator(.forward);
     while (it.next()) |device| {
-        if (device.seat != self.seat) continue;
+        if (device.seat != group.seat) continue;
         if (device.wlr_device.type != .keyboard) continue;
 
-        if (mem.eql(u8, _id, device.identifier)) {
+        if (mem.eql(u8, new_id, device.identifier)) {
             log.debug("found existing matching keyboard; adding to group", .{});
 
             const wlr_keyboard = device.wlr_device.device.keyboard;
-            if (!self.group.addKeyboard(wlr_keyboard)) continue; // wlroots logs its own errors.
+            if (!group.wlr_group.addKeyboard(wlr_keyboard)) {
+                // wlroots logs an error message to explain why this failed.
+                continue;
+            }
         }
 
         // Continue, because we may have more than one device with the exact
-        // same identifier. That is in fact the reason for the keyboard group
+        // same identifier. That is in fact one reason for the keyboard group
         // feature to exist in the first place.
     }
 }
 
-pub fn containsIdentifier(self: *Self, id: []const u8) bool {
+pub fn removeIdentifier(group: *KeyboardGroup, id: []const u8) !void {
-    for (self.keyboard_identifiers.items) |ki| {
+    if (group.identifiers.fetchRemove(id)) |kv| {
-        if (mem.eql(u8, ki, id)) return true;
+        util.gpa.free(kv.key);
     }
-    return false;
-}
 
-pub fn addKeyboard(self: *Self, keyboard: *Keyboard) !void {
+    var it = server.input_manager.devices.iterator(.forward);
-    debug.assert(keyboard.provider != .group);
+    while (it.next()) |device| {
-    const wlr_keyboard = keyboard.provider.device.wlr_device.device.keyboard;
+        if (device.seat != group.seat) continue;
-    log.debug("keyboard group '{s}' adding keyboard: '{s}'", .{ self.name, keyboard.provider.device.identifier });
+        if (device.wlr_device.type != .keyboard) continue;
-    if (!self.group.addKeyboard(wlr_keyboard)) {
+
-        log.err("failed to add keyboard to group", .{});
+        if (mem.eql(u8, device.identifier, id)) {
-        return error.OutOfMemory;
+            const wlr_keyboard = device.wlr_device.device.keyboard;
+            assert(wlr_keyboard.group == group.wlr_group);
+            group.wlr_group.removeKeyboard(wlr_keyboard);
+        }
     }
 }

river/Seat.zig

@@ -32,6 +32,7 @@ const DragIcon = @import("DragIcon.zig");
 const InputDevice = @import("InputDevice.zig");
 const InputManager = @import("InputManager.zig");
 const Keyboard = @import("Keyboard.zig");
+const KeyboardGroup = @import("KeyboardGroup.zig");
 const KeycodeSet = @import("KeycodeSet.zig");
 const LayerSurface = @import("LayerSurface.zig");
 const Mapping = @import("Mapping.zig");
@@ -41,7 +42,6 @@ const Switch = @import("Switch.zig");
 const View = @import("View.zig");
 const ViewStack = @import("view_stack.zig").ViewStack;
 const XwaylandOverrideRedirect = @import("XwaylandOverrideRedirect.zig");
-const KeyboardGroup = @import("KeyboardGroup.zig");
 
 const log = std.log.scoped(.seat);
 const PointerConstraint = @import("PointerConstraint.zig");
@@ -128,6 +128,10 @@ pub fn deinit(self: *Self) void {
     self.cursor.deinit();
     self.mapping_repeat_timer.remove();
 
+    while (self.keyboard_groups.first) |node| {
+        node.data.destroy();
+    }
+
     while (self.focus_stack.first) |node| {
         self.focus_stack.remove(node);
         util.gpa.destroy(node);
@@ -478,18 +482,7 @@ fn tryAddDevice(self: *Self, wlr_device: *wlr.InputDevice) !void {
 
             try keyboard.init(self, wlr_device);
 
-            // Add this keyboard to a keyboard group, if the group contains a
+            self.wlr_seat.setKeyboard(keyboard.device.wlr_device);
-            // matching identifier and if the keyboard isn't a group itself.
-            if (keyboard.provider == .device) {
-                var it = self.keyboard_groups.first;
-                while (it) |node| : (it = node.next) {
-                    if (node.data.containsIdentifier(keyboard.provider.device.identifier)) {
-                        try node.data.addKeyboard(keyboard);
-                        break;
-                    }
-                }
-            }
-
             if (self.wlr_seat.keyboard_state.focused_surface) |wlr_surface| {
                 self.wlr_seat.keyboardNotifyClearFocus();
                 self.keyboardNotifyEnter(wlr_surface);
@@ -522,7 +515,6 @@ pub fn updateCapabilities(self: *Self) void {
 
     var it = server.input_manager.devices.iterator(.forward);
     while (it.next()) |device| {
-        log.debug(">>>> '{s}'", .{device.identifier});
         if (device.seat == self) {
             switch (device.wlr_device.type) {
                 .keyboard => capabilities.keyboard = true,

river/command.zig

@@ -89,9 +89,10 @@ const command_impls = std.ComptimeStringMap(
         .{ "unmap-switch",              @import("command/map.zig").unmapSwitch },
         .{ "xcursor-theme",             @import("command/xcursor_theme.zig").xcursorTheme },
         .{ "zoom",                      @import("command/zoom.zig").zoom },
-        .{ "keyboard-group-create",     @import("command/keyboard_group.zig").keyboardGroupCreate},
+        .{ "keyboard-group-create",     @import("command/keyboard_group.zig").keyboardGroupCreate },
-        .{ "keyboard-group-destroy",    @import("command/keyboard_group.zig").keyboardGroupDestroy},
+        .{ "keyboard-group-destroy",    @import("command/keyboard_group.zig").keyboardGroupDestroy },
-        .{ "keyboard-group-add-keyboard", @import("command/keyboard_group.zig").keyboardGroupAddIdentifier},
+        .{ "keyboard-group-add",        @import("command/keyboard_group.zig").keyboardGroupAdd },
+        .{ "keyboard-group-remove",     @import("command/keyboard_group.zig").keyboardGroupRemove },
     },
 );
 // zig fmt: on

river/command/keyboard_group.zig

@@ -32,19 +32,13 @@ pub fn keyboardGroupCreate(
     if (args.len < 2) return Error.NotEnoughArguments;
     if (args.len > 2) return Error.TooManyArguments;
 
-    var it = seat.keyboard_groups.first;
+    if (keyboardGroupFromName(seat, args[1]) != null) {
-    while (it) |node| : (it = node.next) {
+        const msg = try util.gpa.dupe(u8, "error: failed to create keybaord group: group of same name already exists\n");
-        if (mem.eql(u8, node.data.name, args[1])) {
+        out.* = msg;
-            const msg = try util.gpa.dupe(u8, "error: failed to create keybaord group: group of same name already exists\n");
+        return;
-            out.* = msg;
-            return;
-        }
     }
 
-    const node = try util.gpa.create(std.TailQueue(KeyboardGroup).Node);
+    try KeyboardGroup.create(seat, args[1]);
-    errdefer util.gpa.destroy(node);
-    try node.data.init(seat, args[1]);
-    seat.keyboard_groups.append(node);
 }
 
 pub fn keyboardGroupDestroy(
@@ -54,18 +48,15 @@ pub fn keyboardGroupDestroy(
 ) Error!void {
     if (args.len < 2) return Error.NotEnoughArguments;
     if (args.len > 2) return Error.TooManyArguments;
-    const kg = keyboardGroupFromName(seat, args[1]) orelse {
+    const group = keyboardGroupFromName(seat, args[1]) orelse {
         const msg = try util.gpa.dupe(u8, "error: no keyboard group with that name exists\n");
         out.* = msg;
         return;
     };
-    kg.deinit();
+    group.destroy();
-    const node = @fieldParentPtr(std.TailQueue(KeyboardGroup).Node, "data", kg);
-    seat.keyboard_groups.remove(node);
-    util.gpa.destroy(node);
 }
 
-pub fn keyboardGroupAddIdentifier(
+pub fn keyboardGroupAdd(
     seat: *Seat,
     args: []const [:0]const u8,
     out: *?[]const u8,
@@ -73,12 +64,28 @@ pub fn keyboardGroupAddIdentifier(
     if (args.len < 3) return Error.NotEnoughArguments;
     if (args.len > 3) return Error.TooManyArguments;
 
-    const kg = keyboardGroupFromName(seat, args[1]) orelse {
+    const group = keyboardGroupFromName(seat, args[1]) orelse {
         const msg = try util.gpa.dupe(u8, "error: no keyboard group with that name exists\n");
         out.* = msg;
         return;
     };
-    try kg.addKeyboardIdentifier(args[2]);
+    try group.addIdentifier(args[2]);
+}
+
+pub fn keyboardGroupRemove(
+    seat: *Seat,
+    args: []const [:0]const u8,
+    out: *?[]const u8,
+) Error!void {
+    if (args.len < 3) return Error.NotEnoughArguments;
+    if (args.len > 3) return Error.TooManyArguments;
+
+    const group = keyboardGroupFromName(seat, args[1]) orelse {
+        const msg = try util.gpa.dupe(u8, "error: no keyboard group with that name exists\n");
+        out.* = msg;
+        return;
+    };
+    try group.removeIdentifier(args[2]);
 }
 
 fn keyboardGroupFromName(seat: *Seat, name: []const u8) ?*KeyboardGroup {