fix: respect user's minimumKeyTime for modifier-only hotkeys#241
Open
monotykamary wants to merge 1 commit into
Open
fix: respect user's minimumKeyTime for modifier-only hotkeys#241monotykamary wants to merge 1 commit into
monotykamary wants to merge 1 commit into
Conversation
Remove the hardcoded 0.3s floor that was enforced for modifier-only hotkeys regardless of the user's minimumKeyTime setting. The slider in Settings already allows values down to 0.0s, but the max(minimumKeyTime, 0.3) calls silently overrode it. Users with obscure hotkeys (e.g. right Option) that don't conflict with OS shortcuts should be able to set a lower threshold for faster response. The default of 0.2s still provides a reasonable guard against accidental triggers. Changes: - RecordingDecisionEngine.decide(): use minimumKeyTime directly instead of max(minimumKeyTime, modifierOnlyMinimumDuration=0.3) - HotKeyProcessor.process(keyEvent): same for extra key press threshold - HotKeyProcessor.processMouseClick(): same for mouse click cancellation - Updated tests and doc comments accordingly The modifierOnlyMinimumDuration constant is kept for reference but no longer enforced as a floor.
Contributor
📝 WalkthroughWalkthroughThis PR consolidates how modifier-only hotkeys enforce minimum key-press duration. Instead of using a fixed ChangesModifier-only hotkey minimum duration consolidation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 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)
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@HexCore/Sources/HexCore/Constants.swift`:
- Line 50: Update the misleading comments in Constants.swift so they match the
hotkey timing contract: state that modifier-only hotkeys use a fixed 0.3s safety
threshold (not the user's minimumKeyTime), while regular hotkeys use the user's
minimumKeyTime; reference the HotKeyProcessor timing behavior and the
minimumKeyTime symbol in the comment to make the distinction explicit and ensure
consistency with the modifier-only handling described in HotKeyProcessor.swift.
In `@HexCore/Sources/HexCore/Logic/HotKeyProcessor.swift`:
- Around line 46-47: The discard threshold for modifier-only hotkeys was changed
to use minimumKeyTime directly; revert this by enforcing a 0.3s safety floor for
modifier-only hotkeys: compute the threshold as max(minimumKeyTime, 0.3) (or
otherwise ensure it is at least 0.3s) wherever discard/ignore decisions are made
(e.g., the mouse-click cancellation path and the non-matching-chord cancellation
path in HotKeyProcessor.swift); update the three other occurrences called out in
the comment so modifier-only hotkeys use the 0.3s floor while regular hotkeys
continue to use minimumKeyTime.
In `@HexCore/Sources/HexCore/Logic/RecordingDecision.swift`:
- Around line 69-73: The discard logic now uses effectiveMinimum =
context.minimumKeyTime directly which allows sub-0.3s modifier-only recordings;
change the computation so that when the recording/hotkey is modifier-only (check
the appropriate boolean used in this file, e.g., isModifierOnly or modifierOnly
flag on the recording/hotkey), effectiveMinimum becomes
max(context.minimumKeyTime, 0.3) instead of just context.minimumKeyTime; update
the assignment in RecordingDecision.swift where effectiveMinimum is set and
ensure any downstream comparisons use this capped value.
In `@HexCore/Tests/HexCoreTests/HotKeyProcessorTests.swift`:
- Around line 741-743: Tests wrongly assume modifier-only hotkeys respect the
user-configured minimumKeyTime (e.g., 0.2s); update assertions to enforce the
fixed modifier-only safety floor of 0.3s. Locate the tests that call
makeContext(hotkey: HotKey(key: nil, modifiers: [...] ), duration: ...) and
assert via RecordingDecisionEngine.decide(...) and change expectations so
durations < 0.3s result in the non-transcription decision and durations >= 0.3s
result in .proceedToTranscription, ensuring all occurrences (around the
makeContext/HotKey usage at the referenced ranges) follow the 0.3s threshold
rather than the user minimumKeyTime.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 65cd24d6-e748-4bd0-b821-e36062dc233e
📒 Files selected for processing (4)
HexCore/Sources/HexCore/Constants.swiftHexCore/Sources/HexCore/Logic/HotKeyProcessor.swiftHexCore/Sources/HexCore/Logic/RecordingDecision.swiftHexCore/Tests/HexCoreTests/HotKeyProcessorTests.swift
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Remove the hardcoded 0.3s floor that was enforced for modifier-only hotkeys regardless of the user's
minimumKeyTimesetting. The Settings slider already allows values down to 0.0s, butmax(minimumKeyTime, 0.3)silently overrode it.Users with obscure hotkeys (e.g. right Option) that don't conflict with OS shortcuts should be able to set a lower threshold for faster response. The default of 0.2s still provides a reasonable guard against accidental triggers.
Changes
RecordingDecisionEngine.decide()— useminimumKeyTimedirectly instead ofmax(minimumKeyTime, modifierOnlyMinimumDuration=0.3)HotKeyProcessor.process(keyEvent:)— same for extra key press threshold during holdHotKeyProcessor.processMouseClick()— same for mouse click cancellation thresholdThe
modifierOnlyMinimumDurationconstant is kept for reference but no longer enforced as a floor.Testing
All 62 existing tests pass with updated assertions. No new tests added — existing tests were adjusted to verify that the user's
minimumKeyTimeis respected directly.Summary by CodeRabbit
Bug Fixes
Documentation
Tests