Skip to content

feat: restrict trigger key to modifier keys only#26

Merged
ZhaoChaoqun merged 4 commits into
mainfrom
modifier-keys-only
Apr 4, 2026
Merged

feat: restrict trigger key to modifier keys only#26
ZhaoChaoqun merged 4 commits into
mainfrom
modifier-keys-only

Conversation

@ZhaoChaoqun

Copy link
Copy Markdown
Owner

Summary

  • Only allow modifier keys (Fn/Command/Option/Control/Shift) as trigger keys
  • Regular keys are no longer accepted since listenOnly event taps cannot swallow their events
  • Remove ~90 lines of dead code (keyCodeDisplayName, keyCodeToCharacter, non-modifier matching logic)
  • Auto-migrate old non-modifier key configs back to default Fn

Test plan

  • 录制按键 → 按普通键(字母/数字)无反应,按修饰键正常录制
  • 左右修饰键区分正确(如 Left Command vs Right Command)
  • 恢复默认按钮正常工作
  • 旧版本设置过普通键的用户升级后自动回退到 Fn

🤖 Generated with Claude Code

ZhaoChaoqun and others added 3 commits April 3, 2026 22:31
CGEvent.tapCreate with .defaultTap creates an active filter that
serialises every keystroke through our callback. Any main-thread
latency (DispatchQueue.main.async, lock contention) stalls all
keyboard and mouse input system-wide.

Switch to .listenOnly — we only observe the trigger key, never need
to block or modify events.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
With .listenOnly, the callback return value is ignored by the system —
returning nil no longer swallows events. Remove the two return-nil
branches and the now-unnecessary isModifierKey check so the code
faithfully reflects that we never intercept events.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Only allow Fn, Command, Option, Control, and Shift (left/right) as
trigger keys. Regular keys (letters, numbers, function keys, etc.)
are no longer accepted because listenOnly event taps cannot swallow
their events, causing them to leak into the foreground app.

Changes:
- Remove non-modifier key support from TriggerKeyConfig
- Narrow event mask to flagsChanged only
- Remove dead code: keyCodeDisplayName(), keyCodeToCharacter()
- Remove Carbon.HIToolbox import (no longer needed)
- Add migration: old non-modifier configs fall back to default Fn
- Update UI text to indicate only modifier keys are supported

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
claude[bot]
claude Bot previously requested changes Apr 4, 2026

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

代码审查 — Linus Torvalds 风格

评分:6/10 — 必须修改后重新提交

维度 得分 说明
正确性 1/3 Codable 迁移有破坏性 bug,老用户升级必 crash
简洁性 2/2 删除 ~90 行死代码,干净利落
可读性 2/2 代码自文档化,修饰键逻辑一目了然
可维护性 1/2 迁移逻辑不够健壮
安全性 1/1 无问题

总体评价:

方向完全正确——listenOnly event tap 本来就吞不掉普通键事件,把这条路堵死是务实的选择。删掉 keyCodeDisplayNamekeyCodeToCharacterCarbon.HIToolbox 依赖,减少了不必要的复杂性。

但是,有一个会让老用户升级后直接 crash 到默认值的 Codable 解码 bug,这是不能合并的。详见 inline comment。

Comment thread Sources/KeyMonitor.swift
@claude claude Bot dismissed their stale review April 4, 2026 06:11

重新审查——上一轮的 Codable bug 判断有误,Swift 默认忽略多余 JSON 字段,迁移逻辑是安全的。

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

代码审查 — Linus Torvalds 风格(修正版)

之前的审查已 dismiss——我最初怀疑 Codable 迁移有 bug,但验证后确认 Swift 自动合成的 Decodable 会忽略 JSON 中的多余字段(isModifierKey),解码不会失败。flagMask == 0 的迁移逻辑也正确覆盖了旧版普通键配置的回退。

评分:9/10 — APPROVE

维度 得分 说明
正确性 3/3 迁移逻辑正确,修饰键匹配无误
简洁性 2/2 删除 ~90 行死代码(keyCodeDisplayNamekeyCodeToCharacterCarbon.HIToolbox),净减 95 行
可读性 2/2 matches() 从 if-else 两路分支简化为纯 flagsChanged 单路,一目了然
可维护性 1/2 小扣分:flagMask == 0 作为"旧版非修饰键配置"的判定条件缺少注释,后来的维护者需要考古才能理解
安全性 1/1 无问题

总体评价:

Talk is cheap, 这个 PR 用代码说话——listenOnly event tap 本来就吞不掉普通键事件,继续支持普通键触发是自欺欺人。把这条路堵死、删掉对应的死代码,是正确且务实的决定。

亮点:

  • event mask 精简:从 flagsChanged | keyDown | keyUp 缩减为只监听 flagsChanged,减少了不必要的事件回调开销
  • 删掉 Carbon.HIToolbox 依赖UCKeyTranslate 那坨 unsafeBitCast 代码本身就是技术债,删得好
  • 迁移策略简洁有效:利用 flagMask == 0 区分旧版普通键配置,静默回退到 Fn,用户无感知
  • UI 文案同步更新:「请按下任意键」→「请按下修饰键」,footer 说明也补充了支持的修饰键列表

代码干净,逻辑清晰,没有过度工程化。可以合并。

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

代码审查 — Linus Torvalds 风格(第三轮)

上次审查后唯一的新 commit(fefcd2a)补充了 flagMask == 0 迁移逻辑的注释——正是上轮扣分项。已无遗留问题。

评分:9/10 — APPROVE

维度 得分 说明
正确性 3/3 迁移逻辑正确,Codable 向后兼容,修饰键匹配无误
简洁性 2/2 净减 ~95 行死代码,干净利落
可读性 2/2 迁移注释已补充完整,代码自文档化
可维护性 1/2 小扣分:captureKeyConfig 的修饰键优先级(Fn > Command > Option > Control > Shift)是隐式的——同时按住多个修饰键录制时取最先匹配的,行为合理但不显而易见
安全性 1/1 无问题

这个 PR 做对了一件事:既然 listenOnly event tap 吞不掉普通键事件,那就别假装能支持它们。删掉 Carbon.HIToolboxUCKeyTranslate 那坨 unsafeBitCast 代码、还有 90 行 switch-case 键名映射表——这些都是给一个不该存在的功能写的支撑代码。

可以合并。

@ZhaoChaoqun ZhaoChaoqun merged commit 367c819 into main Apr 4, 2026
5 checks passed
@ZhaoChaoqun ZhaoChaoqun deleted the modifier-keys-only branch April 4, 2026 06:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant