Tap the hold shortcut twice to latch into toggle mode - #291
Open
gfrankgva wants to merge 1 commit into
Open
Conversation
Holding a key down for a long dictation is uncomfortable, and the existing way to latch — press the extra modifier of the toggle shortcut while still holding — is hard to discover and awkward on a modifier-only hold key such as Fn. Two quick taps of the hold shortcut now start a latched (toggle) session instead of a second hold. Releasing that second tap arms the stop, and the next tap ends it, so the whole gesture uses one key. Notes on the implementation: - The gap is measured between the release of the first tap and the press of the second, default 400 ms, with an injectable clock so the window is testable without sleeping. - `lastHoldReleaseAt` deliberately survives `reset()`: the gap spans two sessions and `reset()` runs between them when the first tap's clip is committed. - The latch is allowed while the first tap's near-empty clip is still transcribing. Without that exception the `isTranscribing` guard swallows the second tap and the speaker gets nothing. - A session latched this way is also stopped by the hold shortcut, since that is the only key the speaker touched. Sessions entered through the toggle shortcut keep their existing behaviour exactly. Adds nine tests, including three that pin the current behaviour of plain hold, plain toggle, and hold-then-toggle so this cannot regress. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughAdds configurable double-tap latching for hold shortcuts, tracks hold-origin toggle sessions, and adds deterministic standalone tests with Makefile build and test integration. ChangesShortcut latching
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant HoldShortcut
participant DictationShortcutSessionController
participant InjectableClock
HoldShortcut->>DictationShortcutSessionController: release hold
DictationShortcutSessionController->>InjectableClock: record release time
HoldShortcut->>DictationShortcutSessionController: activate hold
DictationShortcutSessionController->>InjectableClock: check double-tap interval
DictationShortcutSessionController-->>HoldShortcut: enter toggle session
HoldShortcut->>DictationShortcutSessionController: release hold
DictationShortcutSessionController->>DictationShortcutSessionController: arm stopping
HoldShortcut->>DictationShortcutSessionController: activate hold
DictationShortcutSessionController->>DictationShortcutSessionController: stop toggle session
Suggested reviewers: 🚥 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 |
![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: 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 `@Sources/ShortcutCore/DictationShortcutSessionController.swift`:
- Around line 141-143: Update isDoubleTap() to require the interval from
lastHoldReleaseAt to now() to be nonnegative and no greater than
doubleTapWindow, so backward wall-clock adjustments cannot qualify as a double
tap.
🪄 Autofix
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 Plus
Run ID: 24e82486-b473-4769-9db1-0b413336e2d4
📒 Files selected for processing (3)
MakefileSources/ShortcutCore/DictationShortcutSessionController.swiftTests/DictationShortcutSessionControllerTests.swift
Comment on lines
+141
to
+143
| private func isDoubleTap() -> Bool { | ||
| guard doubleTapLatchEnabled, let last = lastHoldReleaseAt else { return false } | ||
| return now().timeIntervalSince(last) <= doubleTapWindow |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Reject negative double-tap intervals.
Date is a wall clock and can move backward. A negative interval currently satisfies <= doubleTapWindow, so a hold activation can incorrectly enter toggle mode after a clock correction. Require an elapsed interval in the range 0...doubleTapWindow.
Proposed fix
private func isDoubleTap() -> Bool {
guard doubleTapLatchEnabled, let last = lastHoldReleaseAt else { return false }
- return now().timeIntervalSince(last) <= doubleTapWindow
+ let elapsed = now().timeIntervalSince(last)
+ return elapsed >= 0 && elapsed <= doubleTapWindow
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| private func isDoubleTap() -> Bool { | |
| guard doubleTapLatchEnabled, let last = lastHoldReleaseAt else { return false } | |
| return now().timeIntervalSince(last) <= doubleTapWindow | |
| private func isDoubleTap() -> Bool { | |
| guard doubleTapLatchEnabled, let last = lastHoldReleaseAt else { return false } | |
| let elapsed = now().timeIntervalSince(last) | |
| return elapsed >= 0 && elapsed <= doubleTapWindow | |
| } |
🤖 Prompt for 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.
In `@Sources/ShortcutCore/DictationShortcutSessionController.swift` around lines
141 - 143, Update isDoubleTap() to require the interval from lastHoldReleaseAt
to now() to be nonnegative and no greater than doubleTapWindow, so backward
wall-clock adjustments cannot qualify as a double tap.
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.
Why
Holding a key down for a long dictation is uncomfortable, and the existing way to latch — press the extra modifier of the toggle shortcut while still holding the hold shortcut — is hard to discover. On a modifier-only hold key such as Fn it is also awkward to perform.
Two quick taps of the hold shortcut now start a latched (toggle) session instead of a second hold, so the whole gesture uses one key: tap tap, speak, tap.
What changed
All of it is in
DictationShortcutSessionController, so both the runtime path inAppStateand the setup wizard'sSetupTestHotkeyHarnessget it without changes.defaultDoubleTapWindow, with an injectable clock so the window is testable without sleeping.lastHoldReleaseAtdeliberately survivesreset(). The gap spans two sessions andreset()runs between them when the first tap's clip is committed, so clearing it there would make the second tap unreachable.isTranscribingguard swallows the second tap and the speaker gets nothing at all. The stray clip is sub-100 ms and already lands in the existing empty-transcript path.doubleTapLatchEnableddefaults to true and is a plain property, so gating it behind a setting later is a one-line change inAppState.Tests
New
Tests/DictationShortcutSessionControllerTests.swift, wired intomake testas a second runner. Nine cases: the double tap itself, the latch surviving the first tap's transcription, the third tap stopping, the second tap's release not stopping, a slow second tap staying an ordinary hold, and the disabled path — plus three that pin the current behaviour of plain hold, plain toggle, and hold-then-toggle so this cannot regress.Built and tested on macOS 15 (Apple silicon).
Note
Written for a user who dictates with Fn and found holding the key uncomfortable over long passages. Happy to put it behind a setting, change the default window, or rename anything to fit your conventions.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes