Fix composite shortcut modifier matching#88
Fix composite shortcut modifier matching#88Dj-Shortcut wants to merge 1 commit intoAlanRockefeller:mainfrom
Conversation
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughModifies keyboard modifier-key matching logic to require exact bitmask inclusion of all required modifiers, rather than any overlap. Adds regression tests for keyboard shortcuts invoking white-balance auto-adjust and batch membership toggle actions with specific modifier combinations. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the 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 |
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
What brokeThe shortcut matcher treated composite modifier shortcuts as a partial match. Because Root causeThe bitmask check only required any overlapping modifier bit instead of all required bits for the mapped shortcut. FixRequire all mapped modifier bits to be present: (modifiers & mapped_modifier) == mapped_modifier
This keeps Ctrl+Shift+B working while preventing Ctrl+B and Shift+B from accidentally matching the composite shortcut.
What I tested
Local verification, since CI is not running on my fork:
python -m pytest faststack/tests/test_auto_adjust_regressions.py -q
................ [100%]
16 passed in 1.15s
After installing PySide6 locally, I also ran the nearby optional tests:
python -m pytest faststack/tests/thumbnail_view/test_selection.py faststack/tests/test_variants.py -q
.................................................. [100%]
50 passed in 2.28s
Notes
Kept this intentionally narrow: no QML changes and no unrelated keybinding refactor.
|
|
Suggestions for this pull request merged into #91 Thanks for your help! |
|
You're welcome |
Summary by CodeRabbit
Bug Fixes
Tests