#1866 fix: deduplicate binds by resolved combo and drop unreachable ones - #1870
Conversation
|
Caution Review failedThe pull request is closed. βΉοΈ Recent review infoβοΈ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: π Files selected for processing (3)
π WalkthroughWalkthroughThe Lua binding layer canonicalizes modifier combinations for deduplication, enables deduplication before registration, corrects the waybar shortcut, and uses keysyms for workspace 11β20 numpad bindings. The changelog records these Hyprland fixes. ChangesKeybinding fixes
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Poem
π₯ Pre-merge checks | β 5β Passed checks (5 passed)
β¨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
π€ Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Configs/.local/share/hypr/lua/hyde/binds.lua`:
- Around line 57-65: Update the modifier canonicalization logic before sorting
in the visible binding formatter so repeated normalized modifiers are collapsed
into a set. Ensure inputs such as CTRL + CTRL + B and CTRL + B produce the same
deduplication ID, while preserving alias normalization, deterministic sorting,
and key handling.
πͺ Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
βΉοΈ Review info
βοΈ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 00634863-8d7e-4b96-b91b-42482894d732
π Files selected for processing (2)
Configs/.local/share/hypr/lua/hyde/binds.luaConfigs/.local/share/hypr/lua/key_binds.lua
|
ready? |
β¦nreachable ones
|
I should alos warn that some of the keybinds are layout specific example 3 finger swipe (touchpad) for scrolling windows. It needs to be specific if we want it up and down and window follows the fingers.. Cna you do a test for that please |
|
Thanks for the review. Pushed one more change from the bot's catch: repeated modifiers are now collapsed before sorting, so |
|
On the gesture point β added it to the suite in #1871 rather than here, since this one is merged. The harness now records The duplicate check is the one that matters for what you described. Hyprland reports an invalid direction or action as a config error, but says nothing when the same swipe is declared twice β the second one just shadows the first. So Right now HyDE ships no gestures at all β the three examples in What I cannot test from the tree is whether the window actually follows the fingers β that is runtime behaviour on real hardware. If you want a rule like "a gesture moving windows must use a specific direction, never a bare axis", say so and I will encode it; I did not want to invent the policy. |

Pull Request
Description
Fixes #1866
Problem
Three defects in the Lua keybinds, all silent:
hyprctl configerrorsstaysempty and the session starts normally.
SUPER + CTRL + Left/Rightcarried two actions at once. Group navigationspelled the combo
CTRL + Left, relative workspace navigation spelled thesame physical combo
CONTROL + LEFT, and both binds stayed live.ALT_R + CONTROL_Rhid the bar.ALT_Ris a keysym, not a modifier, soHyprland dropped it and registered a bare right Control.
code:NNform, so all twenty binds registered with an empty key and azero keycode.
Changes
hyde/binds.luacanonicalize, which reduces a combo to what Hyprland actuallymatches on: modifiers uppercased, aliases folded (
CONTROLtoCTRL,WIN/LOGO/MOD4toSUPER,MOD1toALT), sorted, key last. Thededup key is now the canonical form, so spelling and order no longer hide
a collision.
hyde.binds._activenow stores the exact string a combo was registeredwith instead of
true. Unbinding matches that literal string rather thanthe resolved key and modifiers, so without it dedup could not remove a
bind that was first registered under a different spelling.
key_binds.luahyde.binds.dedup = truemoves from the last line of the file to thefirst. It previously ran after every bind in the file had been registered,
so only user overrides were ever deduplicated.
SUPER + CTRL + arrowsgroup binds. Group navigation stays onSUPER + ALT + arrows, the combo goes to relative workspace navigationalone.
SUPER + CTRL + B.is bound under both keysyms its key can emit: the digit with Num Lock on,
the navigation name with it off.
Verification
Loading
key_binds.luaagainst a stubbedhl/hydeAPI, before and after:The 23 were one duplicate combo, twenty unreachable
code:NNbinds and thetwo spellings of the right Control bind.
Live session on Hyprland 0.56.1 after
hyprctl reload:The bind count difference is accounted for exactly: minus twenty broken
numpad binds, plus forty working ones, minus the two duplicate group binds.
Type of change
Three key combinations change meaning, which users of the previous defaults
will notice: hiding Waybar moves off the right Control key onto
Super + Ctrl + B, andSuper + Ctrl + arrowsstops doing double duty. Bothwere unusable as shipped, so nothing that worked before stops working.
Checklist
Tests for exactly these defects are in #1871, kept separate so this fix stays
reviewable on its own.
Additional context
pre-commit runpasses on the files this branch touches. The repository has244 files with pre-existing end-of-file and trailing-whitespace findings; they
are left alone here rather than folded into this change.
Summary by CodeRabbit
Super + Ctrl + Arrowbehavior to prevent unintended double group/workspace changes.Super + Ctrl + Bto avoid accidental activation.