From 46f77f30dcce06b7af0ec8dff5ae3e4fbc73176f Mon Sep 17 00:00:00 2001 From: Isaac Freund Date: Sat, 29 Mar 2025 10:54:29 +0100 Subject: [PATCH] Seat: put all keyboards in a single group Deprecate and ignore the riverctl commands for creating explicit keyboard groups. In my mind, the only reason to have more than one keyboard group is if different keyboard devices are assigned different keymaps or repeat rates. River does not currently allow such things to be configured however. When river eventually makes it possible to configure different keymaps and repeat rates per keyboard device, there is no reason we can't 100% automatically group keyboards based on the keymap/repeat rate. Exposing this keyboard group abstraction to the user is just bad UX. Failing to group keyboards automatically also creates confusing/buggy behavior for the user if the hardware, for example, exposes some of the the XF86 buttons on a laptop as a separate keyboard device from the main keyboard. Creating keybindings for these XF86 buttons that use modifiers doesn't work by default, but there's no reason it shouldn't just work. Closes: https://codeberg.org/river/river/issues/1138 --- completions/bash/riverctl | 4 - completions/fish/riverctl.fish | 5 -- completions/zsh/_riverctl | 5 -- doc/riverctl.1.scd | 19 ----- river/Keyboard.zig | 13 +-- river/KeyboardGroup.zig | 141 ------------------------------- river/Seat.zig | 10 +-- river/command/keyboard_group.zig | 78 ++--------------- 8 files changed, 15 insertions(+), 260 deletions(-) delete mode 100644 river/KeyboardGroup.zig diff --git a/completions/bash/riverctl b/completions/bash/riverctl index a889d4c..891f13c 100644 --- a/completions/bash/riverctl +++ b/completions/bash/riverctl @@ -4,10 +4,6 @@ function __riverctl_completion () if [ "${COMP_CWORD}" -eq 1 ] then OPTS=" \ - keyboard-group-create \ - keyboard-group-destroy \ - keyboard-group-add \ - keyboard-group-remove \ keyboard-layout \ keyboard-layout-file \ close \ diff --git a/completions/fish/riverctl.fish b/completions/fish/riverctl.fish index 50458b4..70aceb7 100644 --- a/completions/fish/riverctl.fish +++ b/completions/fish/riverctl.fish @@ -72,11 +72,6 @@ complete -c riverctl -n '__fish_riverctl_complete_arg 1' -a 'hide-cursor' complete -c riverctl -n '__fish_riverctl_complete_arg 1' -a 'set-repeat' -d 'Set the keyboard repeat rate and repeat delay' complete -c riverctl -n '__fish_riverctl_complete_arg 1' -a 'set-cursor-warp' -d 'Set the cursor warp mode' complete -c riverctl -n '__fish_riverctl_complete_arg 1' -a 'xcursor-theme' -d 'Set the xcursor theme' -# Keyboardgroups -complete -c riverctl -n '__fish_riverctl_complete_arg 1' -a 'keyboard-group-create' -d 'Create a keyboard group' -complete -c riverctl -n '__fish_riverctl_complete_arg 1' -a 'keyboard-group-destroy' -d 'Destroy a keyboard group' -complete -c riverctl -n '__fish_riverctl_complete_arg 1' -a 'keyboard-group-add' -d 'Add a keyboard to a keyboard group' -complete -c riverctl -n '__fish_riverctl_complete_arg 1' -a 'keyboard-group-remove' -d 'Remove a keyboard from a keyboard group' complete -c riverctl -n '__fish_riverctl_complete_arg 1' -a 'keyboard-layout' -d 'Set the keyboard layout' complete -c riverctl -n '__fish_riverctl_complete_arg 1' -a 'keyboard-layout-file' -d 'Set the keyboard layout from a file.' diff --git a/completions/zsh/_riverctl b/completions/zsh/_riverctl index ad372d3..8bfd8f0 100644 --- a/completions/zsh/_riverctl +++ b/completions/zsh/_riverctl @@ -62,11 +62,6 @@ _riverctl_commands() 'set-repeat:Set the keyboard repeat rate and repeat delay' 'set-cursor-warp:Set the cursor warp mode.' 'xcursor-theme:Set the xcursor theme' - # Keyboard groups - 'keyboard-group-create:Create a keyboard group' - 'keyboard-group-destroy:Destroy a keyboard group' - 'keyboard-group-add:Add a keyboard to a keyboard group' - 'keyboard-group-remove:Remove a keyboard from a keyboard group' 'keyboard-layout:Set the keyboard layout' 'keyboard-layout-file:Set the keyboard layout from a file' # Input diff --git a/doc/riverctl.1.scd b/doc/riverctl.1.scd index d0476ea..9c1f223 100644 --- a/doc/riverctl.1.scd +++ b/doc/riverctl.1.scd @@ -454,25 +454,6 @@ matches everything while _\*\*_ and the empty string are invalid. following URL: https://xkbcommon.org/doc/current/keymap-text-format-v1.html -*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* _group_name_ - Destroy the keyboard group with the given name. All attached keyboards - will be released, making them act as separate devices again. - -*keyboard-group-add* _group_name_ _input_device_name_ - Add a keyboard to a keyboard group, identified by the keyboard's - input device name. Any currently connected and future keyboards with - the given name will be added to the group. Simple globbing patterns are - supported, see the rules section for further information on globs. - -*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 decimal vendor id, diff --git a/river/Keyboard.zig b/river/Keyboard.zig index 3c659d8..ef43043 100644 --- a/river/Keyboard.zig +++ b/river/Keyboard.zig @@ -101,16 +101,9 @@ pub fn init(keyboard: *Keyboard, seat: *Seat, wlr_device: *wlr.InputDevice) !voi // wlroots will log a more detailed error if this fails. if (!wlr_keyboard.setKeymap(server.config.keymap)) return error.OutOfMemory; - // Add to keyboard-group, if applicable. - var group_it = seat.keyboard_groups.first; - outer: while (group_it) |group_node| : (group_it = group_node.next) { - for (group_node.data.globs.items) |glob| { - if (globber.match(glob, keyboard.device.identifier)) { - // wlroots will log an error if this fails explaining the reason. - _ = group_node.data.wlr_group.addKeyboard(wlr_keyboard); - break :outer; - } - } + if (wlr.KeyboardGroup.fromKeyboard(wlr_keyboard) == null) { + // wlroots will log an error on failure + _ = seat.keyboard_group.addKeyboard(wlr_keyboard); } wlr_keyboard.setRepeatInfo(server.config.repeat_rate, server.config.repeat_delay); diff --git a/river/KeyboardGroup.zig b/river/KeyboardGroup.zig deleted file mode 100644 index 5f79608..0000000 --- a/river/KeyboardGroup.zig +++ /dev/null @@ -1,141 +0,0 @@ -// This file is part of river, a dynamic tiling wayland compositor. -// -// Copyright 2022 The River Developers -// -// This program is free software: you can redistribute it and/or modify -// it under the terms of the GNU General Public License as published by -// the Free Software Foundation, version 3. -// -// This program is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. -// -// You should have received a copy of the GNU General Public License -// along with this program. If not, see . - -const KeyboardGroup = @This(); - -const std = @import("std"); -const assert = std.debug.assert; -const mem = std.mem; - -const globber = @import("globber"); -const wlr = @import("wlroots"); -const wl = @import("wayland").server.wl; -const xkb = @import("xkbcommon"); - -const log = std.log.scoped(.keyboard_group); - -const server = &@import("main.zig").server; -const util = @import("util.zig"); - -const Seat = @import("Seat.zig"); -const Keyboard = @import("Keyboard.zig"); - -seat: *Seat, -wlr_group: *wlr.KeyboardGroup, -name: []const u8, -globs: std.ArrayListUnmanaged([]const u8) = .{}, - -pub fn create(seat: *Seat, name: []const u8) !void { - log.debug("new keyboard group: '{s}'", .{name}); - - const node = try util.gpa.create(std.TailQueue(KeyboardGroup).Node); - errdefer util.gpa.destroy(node); - - const wlr_group = try wlr.KeyboardGroup.create(); - errdefer wlr_group.destroy(); - - const owned_name = try util.gpa.dupe(u8, name); - errdefer util.gpa.free(owned_name); - - node.data = .{ - .wlr_group = wlr_group, - .name = owned_name, - .seat = seat, - }; - - seat.addDevice(&wlr_group.keyboard.base); - seat.keyboard_groups.append(node); -} - -pub fn destroy(group: *KeyboardGroup) void { - log.debug("destroying keyboard group: '{s}'", .{group.name}); - - util.gpa.free(group.name); - - for (group.globs.items) |glob| { - util.gpa.free(glob); - } - group.globs.deinit(util.gpa); - - group.wlr_group.destroy(); - - const node: *std.TailQueue(KeyboardGroup).Node = @fieldParentPtr("data", group); - group.seat.keyboard_groups.remove(node); - util.gpa.destroy(node); -} - -pub fn addIdentifier(group: *KeyboardGroup, new_id: []const u8) !void { - for (group.globs.items) |glob| { - if (mem.eql(u8, glob, new_id)) return; - } - - log.debug("keyboard group '{s}' adding identifier: '{s}'", .{ group.name, new_id }); - - const owned_id = try util.gpa.dupe(u8, new_id); - errdefer util.gpa.free(owned_id); - - // Glob is validated in the command handler. - try group.globs.append(util.gpa, owned_id); - errdefer { - // Not used now, but if at any point this function is modified to that - // it may return an error after the glob pattern is added to the list, - // the list will have a pointer to freed memory in its last position. - _ = group.globs.pop(); - } - - // Add any existing matching keyboards to the group. - var it = server.input_manager.devices.iterator(.forward); - while (it.next()) |device| { - if (device.seat != group.seat) continue; - if (device.wlr_device.type != .keyboard) continue; - - if (globber.match(device.identifier, new_id)) { - log.debug("found existing matching keyboard; adding to group", .{}); - - if (!group.wlr_group.addKeyboard(device.wlr_device.toKeyboard())) { - // 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 one reason for the keyboard group - // feature to exist in the first place. - } -} - -pub fn removeIdentifier(group: *KeyboardGroup, id: []const u8) !void { - for (group.globs.items, 0..) |glob, index| { - if (mem.eql(u8, glob, id)) { - _ = group.globs.orderedRemove(index); - break; - } - } else { - return; - } - - var it = server.input_manager.devices.iterator(.forward); - while (it.next()) |device| { - if (device.seat != group.seat) continue; - if (device.wlr_device.type != .keyboard) continue; - - if (globber.match(device.identifier, id)) { - const wlr_keyboard = device.wlr_device.toKeyboard(); - assert(wlr_keyboard.group == group.wlr_group); - group.wlr_group.removeKeyboard(wlr_keyboard); - } - } -} diff --git a/river/Seat.zig b/river/Seat.zig index bb8e827..0dc0376 100644 --- a/river/Seat.zig +++ b/river/Seat.zig @@ -33,7 +33,6 @@ const InputDevice = @import("InputDevice.zig"); const InputManager = @import("InputManager.zig"); const InputRelay = @import("InputRelay.zig"); const Keyboard = @import("Keyboard.zig"); -const KeyboardGroup = @import("KeyboardGroup.zig"); const LayerSurface = @import("LayerSurface.zig"); const LockSurface = @import("LockSurface.zig"); const Mapping = @import("Mapping.zig"); @@ -84,7 +83,7 @@ mapping_repeat_timer: *wl.EventSource, /// Currently repeating mapping, if any repeating_mapping: ?*const Mapping = null, -keyboard_groups: std.TailQueue(KeyboardGroup) = .{}, +keyboard_group: *wlr.KeyboardGroup, /// Currently focused output. Null only when there are no outputs at all. focused_output: ?*Output = null, @@ -121,12 +120,15 @@ pub fn init(seat: *Seat, name: [*:0]const u8) !void { .cursor = undefined, .relay = undefined, .mapping_repeat_timer = mapping_repeat_timer, + .keyboard_group = try wlr.KeyboardGroup.create(), }; seat.wlr_seat.data = @intFromPtr(seat); try seat.cursor.init(seat); seat.relay.init(); + try seat.tryAddDevice(&seat.keyboard_group.keyboard.base); + seat.wlr_seat.events.request_set_selection.add(&seat.request_set_selection); seat.wlr_seat.events.request_start_drag.add(&seat.request_start_drag); seat.wlr_seat.events.start_drag.add(&seat.start_drag); @@ -142,9 +144,7 @@ pub fn deinit(seat: *Seat) void { seat.cursor.deinit(); seat.mapping_repeat_timer.remove(); - while (seat.keyboard_groups.first) |node| { - node.data.destroy(); - } + seat.keyboard_group.destroy(); seat.request_set_selection.link.remove(); seat.request_start_drag.link.remove(); diff --git a/river/command/keyboard_group.zig b/river/command/keyboard_group.zig index 442fbdc..95a8153 100644 --- a/river/command/keyboard_group.zig +++ b/river/command/keyboard_group.zig @@ -24,77 +24,13 @@ const util = @import("../util.zig"); const Error = @import("../command.zig").Error; const Seat = @import("../Seat.zig"); -const KeyboardGroup = @import("../KeyboardGroup.zig"); -pub fn keyboardGroupCreate( - seat: *Seat, - args: []const [:0]const u8, - out: *?[]const u8, -) Error!void { - if (args.len < 2) return Error.NotEnoughArguments; - if (args.len > 2) return Error.TooManyArguments; +pub const keyboardGroupCreate = keyboardGroupDeprecated; +pub const keyboardGroupDestroy = keyboardGroupDeprecated; +pub const keyboardGroupAdd = keyboardGroupDeprecated; +pub const keyboardGroupRemove = keyboardGroupDeprecated; - if (keyboardGroupFromName(seat, args[1]) != null) { - const msg = try util.gpa.dupe(u8, "error: failed to create keybaord group: group of same name already exists\n"); - out.* = msg; - return; - } - - try KeyboardGroup.create(seat, args[1]); -} - -pub fn keyboardGroupDestroy( - seat: *Seat, - args: []const [:0]const u8, - out: *?[]const u8, -) Error!void { - if (args.len < 2) return Error.NotEnoughArguments; - if (args.len > 2) 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; - }; - group.destroy(); -} - -pub fn keyboardGroupAdd( - 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 globber.validate(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 { - var it = seat.keyboard_groups.first; - while (it) |node| : (it = node.next) { - if (mem.eql(u8, node.data.name, name)) return &node.data; - } - return null; +fn keyboardGroupDeprecated(_: *Seat, _: []const [:0]const u8, out: *?[]const u8) Error!void { + out.* = try util.gpa.dupe(u8, "warning: explicit keyboard groups are deprecated, " ++ + "all keyboards are now automatically added to a single group\n"); }