feat: add custom trigger key with any-key recording#21
Conversation
Allow users to configure any keyboard key (modifier or regular) as the push-to-talk trigger, replacing the fixed Fn-only approach. Adds a "record key" button in Settings that captures the next keypress as the new trigger key. Default remains Fn. - Rewrite KeyMonitor with TriggerKeyConfig struct (Codable, any key) - Add key recording mode with captureKeyConfig() and UCKeyTranslate - Replace fixed Picker with "press any key to record" UI in Settings - Dynamic menu bar text showing current trigger key name - Notification-based coordination between Settings UI and KeyMonitor Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
🔥 Linus 审查报告
总评:这是一个解决真实问题的 PR——外接键盘 Fn 不可用,用户需要自定义触发键。方向正确,实现也够务实。但有几个并发安全和崩溃风险需要注意。
评分
| 维度 | 分值 | 说明 |
|---|---|---|
| 正确性 | 2/3 | isRecordingKey 线程安全问题;keyCodeToCharacter 有 force unwrap 崩溃风险;onKeyRecorded 回调链有递归风险 |
| 简洁性 | 2/2 | 改动范围合理,没有过度工程化 |
| 可读性 | 2/2 | TriggerKeyConfig 结构清晰,matches() 返回 Bool? 的三态设计干净利落 |
| 可维护性 | 1.5/2 | NotificationCenter 通知链有点多(4个通知名),但对这个场景可以接受 |
| 安全性 | 1/1 | 无敏感数据泄露风险 |
总分:8.5/10 — COMMENT,代码方向正确,有几个需要注意的点但不至于阻塞合并。
Talk is cheap. 具体问题见 inline comments。
- Guard let source from TISCopyCurrentKeyboardLayoutInputSource to avoid force unwrap crash - Guard let UnicodeScalar to avoid force unwrap crash - Protect isRecordingKey with OSAllocatedUnfairLock for thread safety between CGEvent tap callback and main thread - Add safety comments on restartWithNewTriggerKey and handleTriggerKeyChanged about async requirement Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
🔥 Linus 审查报告(第二轮)
上一轮提到的四个问题全部修复了:keyCodeToCharacter 的 force unwrap 改成了 guard let,isRecordingKey 加上了 OSAllocatedUnfairLock,restartWithNewTriggerKey 也加上了线程安全注释。干得不错,说明你在认真对待代码审查。
但我又发现了一个问题——不是小问题,是会让录音状态机卡死的那种。
评分
| 维度 | 分值 | 说明 |
|---|---|---|
| 正确性 | 2.5/3 | restartWithNewTriggerKey 在录音进行中被调用会导致状态机卡死 |
| 简洁性 | 2/2 | 改动范围控制得当 |
| 可读性 | 2/2 | TriggerKeyConfig 设计清晰,matches() 返回 Bool? 三态很干净 |
| 可维护性 | 1.5/2 | NotificationCenter 通知链多了点,但对这个场景可以接受 |
| 安全性 | 1/1 | 无安全风险 |
总分:9/10 — APPROVE。上轮的问题全部修了,新发现的状态机卡死风险需要注意但不阻塞合并(触发条件需要精确时序)。
具体见 inline comment。
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
🔥 Linus 审查报告(第三轮)
三轮审查,三个 commit。上一轮指出的状态机卡死风险也修了——restartWithNewTriggerKey() 现在在 reset 之前先调 onKeyUp?(),正确地将录音状态机推到 .flushing,不会卡死。
这轮仔细过了一遍最终代码,没有发现新的问题。 所有之前标记的问题都已修复:
- ✅
keyCodeToCharacter的 force unwrap →guard let - ✅
isRecordingKey线程安全 →OSAllocatedUnfairLock - ✅
restartWithNewTriggerKey异步调用约束 → 注释明确标注 - ✅ 录音中切换触发键导致状态机卡死 → 先调
onKeyUp?()再 reset
代码本身解决的也是一个真实的问题——外接机械键盘的 Fn 被固件拦截,macOS 根本收不到事件。用「按任意键录制」的方式让用户选择替代键,比提供一个固定下拉菜单要优雅得多。TriggerKeyConfig 的 matches() 返回 Bool? 三态设计干净利落——nil 表示「不是我的事」,true/false 表示「是我的事,按下/松开」。
评分
| 维度 | 分值 | 说明 |
|---|---|---|
| 正确性 | 3/3 | 上轮所有问题已修复,状态机边界处理正确 |
| 简洁性 | 2/2 | 351 行增量,没有过度工程化 |
| 可读性 | 2/2 | TriggerKeyConfig 自文档化,matches() 语义清晰 |
| 可维护性 | 1.5/2 | 4 个 NotificationCenter 通知名有点多,但对跨组件协调来说可以接受 |
| 安全性 | 1/1 | 无安全风险 |
总分:9.5/10 — APPROVE。Talk is cheap, 代码说明一切。合并吧。
Summary
TriggerKeyConfigstruct supporting any keyboard key (modifier or regular) as push-to-talk triggerMotivation
Filco 67-key mechanical keyboard (and many other external keyboards) intercept Fn at firmware level — macOS never sees the keypress. Users need to choose an alternative trigger key. Rather than offering a fixed list, this supports any key via a "press to record" interaction.
Changes
KeyMonitor.swiftTriggerKeyConfig(Codable), key recording mode,captureKeyConfig(),keyCodeDisplayName()with Carbon UCKeyTranslateSettingsGeneralView.swiftTypelessApp.swifttriggerKeyMenuItem, 3 notification observers (changed/recording-requested/cancelled),onKeyRecordedcallbackTest plan
🤖 Generated with Claude Code