feat: add xremap command key - #767
Conversation
|
You do not have enough credits to review this pull request. Please purchase more credits to continue. |
📝 WalkthroughWalkthroughAdded xremap as a flake input and integrated its NixOS module into the system configuration, including uinput/udev support, extensive remapping rules, a systemd user service, Hyprland startup restart, keyd device exclusion, and a conditional Falcon module inclusion. Changes
Sequence Diagram(s)sequenceDiagram
participant UserSession as User Session
participant Hyprland as Hyprland
participant SystemdUser as systemd -- user
participant Xremap as xremap (user service)
participant Udev as udev
participant Keyd as keyd
UserSession->>Hyprland: start session
Hyprland->>SystemdUser: ensure user services active
Hyprland->>Xremap: exec restart xremap (after 1s)
SystemdUser->>Xremap: start/restart xremap.service
Xremap->>Udev: create uinput device & emit udev rules
Udev->>Keyd: devices appear (keyd ignores -1234:5678)
Keyd-->>UserSession: input events (xremap device excluded)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello @shunkakinoki, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request integrates Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
Warning Gemini encountered an error creating the summary. You can try again by commenting |
There was a problem hiding this comment.
Pull request overview
Adds an xremap flake input and enables an xremap-based “Command-style” remapping on the matic NixOS host (including wlroots + device watching) to map a Framework “Hyper” chord into Ctrl-based shortcuts, with Ghostty-specific overrides.
Changes:
- Add
xremap/nix-flakeas a flake input (and lock its dependency closure). - Import the xremap NixOS module on
named-hosts/maticand enable the user service with wlroots + watch. - Configure uinput/udev access and define global + Ghostty-specific key remaps.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
named-hosts/matic/default.nix |
Imports xremap module, enables uinput/udev + xremap user service, and defines the key remapping rules. |
flake.nix |
Adds the xremap flake input. |
flake.lock |
Locks xremap and newly introduced transitive flake inputs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| extraGroups = [ | ||
| "wheel" | ||
| "networkmanager" | ||
| "input" | ||
| "video" | ||
| ]; |
There was a problem hiding this comment.
Adding the user to the input group grants broad read access to /dev/input/event* on many systems (i.e., keylogging capability). If xremap only needs access to /dev/uinput, consider avoiding input group membership and instead granting access just to the uinput node via a dedicated group or uaccess on that device only, to keep privileges minimal.
| hardware.uinput.enable = true; | ||
| boot.kernelModules = [ "uinput" ]; | ||
| services.udev.extraRules = '' | ||
| KERNEL=="uinput", GROUP="input", TAG+="uaccess", MODE:="0660", OPTIONS+="static_node=uinput" |
There was a problem hiding this comment.
The udev rule sets GROUP="input" for the uinput device, which ties the setup to the broad input group and duplicates the uaccess tagging approach. Consider switching this rule to a dedicated group (e.g., uinput) or relying on TAG+="uaccess" alone for the uinput node, so you don’t need to grant the user full input group privileges.
| KERNEL=="uinput", GROUP="input", TAG+="uaccess", MODE:="0660", OPTIONS+="static_node=uinput" | |
| KERNEL=="uinput", TAG+="uaccess", MODE:="0660", OPTIONS+="static_node=uinput" |
| numbers = [ | ||
| "0" | ||
| "1" | ||
| "2" | ||
| "6" | ||
| "7" | ||
| "8" | ||
| "9" | ||
| ]; |
There was a problem hiding this comment.
The numbers list skips "3", "4", and "5", so Hyper+3/4/5 won’t be remapped (and the omission is easy to miss because other sets are complete). If this is meant to cover all number keys, include the missing digits or generate the list programmatically to avoid accidental gaps.
Mesa DescriptionTL;DRIntegrates What changed?
Description generated by Mesa. Update settings |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@named-hosts/matic/default.nix`:
- Around line 117-125: The numbers array named "numbers" is missing "3", "4",
and "5" so Hyper+3/4/5 won't be remapped to Ctrl+3/4/5; either add the missing
entries ("3", "4", "5") to the numbers list in default.nix (the `numbers =
[...]` block) or, if their omission is intentional, add an inline comment next
to the `numbers` declaration (or above it) explaining that 3–5 are reserved
(e.g., for workspace switching in Hyprland) so future readers understand why
they are excluded.
🧹 Nitpick comments (2)
named-hosts/matic/default.nix (2)
146-155: TheHyper+z → C-zoverride inghosttyRemapis redundant.
globalRemapalready maps${hyperPrefix}zto${ctrlPrefix}z(i.e.,C-z). The//merge on line 154 sets it to"C-z", which is identical. If this is intentional documentation of "we considered z and want it to stay as Ctrl+Z in Ghostty", a comment would clarify the intent. Otherwise, it can be removed.
85-178: Consider extracting the xremap configuration to a separate module file.The keyd configuration is imported from
../../config/keyd(line 37), but the xremap config is defined inline (~90 lines). For consistency and to keep this file more manageable, consider extracting the xremap service configuration to a dedicated file (e.g.,../../config/xremap/default.nixor./xremap.nix), passing inusernameandinputsas arguments.This isn't urgent since the config is host-specific, but it would improve readability of this already-large file. Based on learnings, configurations should be kept modular across directories.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="named-hosts/matic/default.nix">
<violation number="1" location="named-hosts/matic/default.nix:161">
P2: Switching xremap to a user service requires the user to have read access to the input devices it watches. The udev rules set GROUP="input" but the user isn’t added to the input group (extraGroups only includes "inputs"), so the user service is likely to fail with permission errors. Add the user to the "input" group or grant uaccess in the udev rules.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
…d-key' into chore/xremap-command-key
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="named-hosts/matic/default.nix">
<violation number="1" location="named-hosts/matic/default.nix:191">
P2: `StartLimitIntervalSec` is a `[Unit]` directive, but it’s currently placed inside `serviceConfig` (the `[Service]` section), so systemd will ignore it. Move it to the unit configuration (e.g., `unitConfig.StartLimitIntervalSec = 0;`) so the start-limit override actually applies.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Changes
Technical Details
Testing
Generated with GitHub Copilot CLI by Claude Sonnet 4.5
Summary by cubic
Adds xremap on matic for a “Command key” feel by mapping the Framework Hyper key to Ctrl. Runs as a wlroots-backed user service that starts after the graphical session, restarts after Hyprland env import, and removes the nerdfonts meta package (keeps nerd-fonts.jetbrains-mono).
New Features
Migration
Written for commit 7fd6611. Summary will update on new commits.