refactor: migrate shortcut system to Preference architecture#14086
refactor: migrate shortcut system to Preference architecture#14086
Conversation
|
Note This comment was translated by Claude. Cherry Studio Shortcut System Refactoring Design
BackgroundThe v1 shortcut system has 5 main problems:
The goal for v2 is clear:
Core ModelThe system consists of 4 layers:
Each has a simple responsibility:
1. Definition Layer
{
key: 'shortcut.feature.quick_assistant.toggle_window',
scope: 'main',
category: 'general',
labelKey: 'mini_window',
global: true,
supportedPlatforms: ['darwin', 'win32']
}Common fields:
2. Storage LayerShortcut preferences only save what users actually change: type PreferenceShortcutType = {
binding: string[]
enabled: boolean
}Defaults are defined in 'shortcut.chat.clear': { enabled: true, binding: ['CommandOrControl', 'L'] }
'shortcut.general.show_main_window': { enabled: false, binding: [] }Static info like 3. Utility LayerThe most important capabilities in
The core function is
For callers, the result is always: type ResolvedShortcut = {
binding: string[]
enabled: boolean
}Runtime BehaviorMain Process
Its implementation focuses on just 3 things:
Current registration logic is incremental diff:
This avoids meaningless full unregister/register. Renderer Process
The settings page only needs to continue handling search, recording, conflict hints, and rendering. Key RulesSeveral rules are critical to stable system operation: Single Source of TruthStatic shortcut information comes only from Preference FirstRuntime state reads/writes only from Preference. Don't introduce Redux, temporary config files, or additional IPC as a second data source. No Binding Means Not TriggerableEven if Hide Shortcuts When Features DisabledCurrently there are two types of feature-state-dependent shortcuts:
When features are disabled, these shortcuts won't register and won't show in settings. Platform Restrictions Apply to Both Registration and Display
Default ShortcutsThe table below keeps only the most essential information: key, default binding, scope, default enabled state. general
chat
topic
feature.selection
Extension MethodAdding a new shortcut requires just 3 steps in principle. 1. Add Default ValueAdd schema default in 'shortcut.chat.regenerate': {
enabled: true,
binding: ['CommandOrControl', 'Shift', 'R']
}2. Add DefinitionAdd static metadata in {
key: 'shortcut.chat.regenerate',
scope: 'renderer',
category: 'chat',
labelKey: 'regenerate'
}3. Use in Target LocationRenderer process: useShortcut('chat.regenerate', () => regenerateLastMessage())Main process: this.handlers.set('shortcut.chat.regenerate', () => {
// ...
})For conditional shortcuts, don't write conditions into the definition layer. Filter at the consumer layer or do early return in the handler. Migration and TestingMigration Status
Current Testing FocusExisting tests primarily cover:
If continuing to expand, prioritize adding these two test types:
SummaryThe core of this refactoring is not "making shortcuts complex," but converging complexity into shared definitions, unified normalization, and clear layering. For daily development, just remember 3 things:
Original ContentCherry Studio 快捷键系统重构设计
背景v1 快捷键系统的主要问题有 5 个：
v2 的目标很明确：
核心模型系统由 4 层组成：
它们各自负责的事情很简单：
1. 定义层
{
key: 'shortcut.feature.quick_assistant.toggle_window',
scope: 'main',
category: 'general',
labelKey: 'mini_window',
global: true,
supportedPlatforms: ['darwin', 'win32']
}常用字段：
2. 存储层快捷键偏好只保存用户真正会改的部分： type PreferenceShortcutType = {
binding: string[]
enabled: boolean
}默认值定义在 'shortcut.chat.clear': { enabled: true, binding: ['CommandOrControl', 'L'] }
'shortcut.general.show_main_window': { enabled: false, binding: [] }这里不存 3. 工具层
核心函数是
对调用侧来说，拿到的始终是： type ResolvedShortcut = {
binding: string[]
enabled: boolean
}运行方式主进程
它的实现重点只有 3 件事：
当前注册逻辑是增量 diff：
这样可以避免无意义的全量 unregister / register。 渲染进程
其中
设置页只需要继续做搜索、录制、冲突提示和渲染。 关键规则有几条规则是这个系统稳定运行的关键： 单一真相源快捷键的静态信息只来自 Preference 优先运行时状态只从 Preference 读写。不要再引入 Redux、临时配置文件或额外 IPC 作为第二数据源。 无绑定即不可触发即使某项 功能关闭时不显示对应快捷键当前有两类依赖功能状态的快捷键：
功能关闭时，这些快捷键不会注册，也不会在设置页显示。 平台限制要同时作用于注册与展示
默认快捷键下表只保留最常用的信息：key、默认绑定、scope、默认启用状态。 general
chat
topic
feature.selection
扩展方式新增一个快捷键，原则上只需要 3 步。 1. 添加默认值在 'shortcut.chat.regenerate': {
enabled: true,
binding: ['CommandOrControl', 'Shift', 'R']
}2. 添加定义在 {
key: 'shortcut.chat.regenerate',
scope: 'renderer',
category: 'chat',
labelKey: 'regenerate'
}3. 在目标位置使用渲染进程： useShortcut('chat.regenerate', () => regenerateLastMessage())主进程： this.handlers.set('shortcut.chat.regenerate', () => {
// ...
})如果是条件型快捷键，不要把条件写进定义层，应该在消费层做过滤或在 handler 内做早返回。 迁移与测试迁移现状
当前测试重点现有测试主要覆盖：
后续如果继续扩展，优先补下面两类测试：
总结这套重构的核心不是"把快捷键做复杂"，而是把复杂度收拢到共享定义、统一归一化和清晰分层里。 对日常开发来说，只需要记住 3 件事：
|
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
kangfenmao
force-pushed
the
refactor/v2/shortcuts
branch
from
b6f7fab to
015feaa
Compare
DeJeune
left a comment
DeJeune
left a comment
There was a problem hiding this comment.
PR #14086 Review: Shortcut System Migration
Overall this is a well-architected migration. The shared definitions layer, incremental globalShortcut registration, and full consumer migration are solid improvements. Below are findings from 4 parallel review agents.
Critical (2)
- C1: Escape key during recording saves
['Escape']as binding instead of canceling (ShortcutSettings.tsx:269) - C2: Conflict detection uses
displayKeysinstead of resolvedbinding(ShortcutSettings.tsx:150)
Warning (6)
- W1:
useHotkeysre-registers on every render due to unstable callback ref (useShortcuts.ts:94) - W2: Rewritten
ShortcutSettings.tsxstill uses antd/styled-components (v2 violation) - W3:
ready-to-showlistener not tracked for lifecycle cleanup (ShortcutService.ts:118) - W4: Preference subscriptions not using
registerDisposable(ShortcutService.ts:49) - W5: Type assertions bypass null safety in
coerceShortcutPreference(utils.ts:124) - W6: Redux actions (
updateShortcut,toggleShortcut,resetShortcutsin store/shortcuts.ts:173) still exported but are now no-ops. If dispatched, state mutates in Redux but does NOT propagate to Preference — silent data inconsistency. Consider removing exports.
Suggestion (3)
- S1:
show_settingshandler too complex for ShortcutService, delegate to WindowService - S2: antd Table render signature parameter mismatch (ShortcutSettings.tsx:390)
- S3:
isValidShortcutdoesn't check for duplicate keys
See inline comments for details.
There was a problem hiding this comment.
Note
This issue/comment/review was translated by Claude.
Overall, the direction of this refactoring is correct (single source of truth + Preference hosting + processor registry are all reasonable), but this PR crams not yet fully clear design and concrete implementation all at once, suggest splitting meow~
Design issues exposed in current review:
- Data model redundancy and not truly implemented — The
binding/rawBinding/hasCustomBindingtriplet has the same values in all scenarios in the current implementation (verified scenario by scenario), the design intention of "distinguishing cleared from unset" only停留在字段名上. The root cause is that the basic design point of "clear vs disable" is not yet clear — since there isenabled, the independent value of "clear" operation is very weak - Placeholder fields lack consumers —
categoryis marked in every definition, but nowhere in the code reads it, the intention is unclear - Type boundaries not converged —
supportedPlatformsusesNodeJS.Platform[]to accept 11 values but actually only uses 3 - Naming questionable —
persistOnBluris essentiallyglobal - Core types missing JSDoc —
ShortcutDefinition/ShortcutPreferenceValueare the single source of truth, but no field has JSDoc - Documentation inconsistent with code — The
definitions.tssection's field descriptions are missing several fields
Suggested split approach:
- PR 1: Pure infrastructure migration — migrate existing shortcuts from Redux/configManager to Preference architecture, do not introduce new data model concepts (no
hasCustomBinding, nobinding/rawBindingseparation), keep current state working - PR 2: On top of PR 1, do data model design, first discuss "clear vs disable", "category usage", "platform type" these design questions in the issue, then implement
- PR 3: UI layer (settings page recording, conflict detection, category grouping, etc.)
This way, each PR's review focus is more concentrated, and avoids the dilemma of "design still under discussion but code already written" leading to either having to live with it or major changes meow~
Specific design / docs / suggestion / question are all written in inline threads 🐱
Original Content
整体上这次重构方向是对的（单一真相源 + Preference 托管 + 处理器注册表都很合理），但这个 PR 把还没完全明朗的设计和具体实现一次性塞进来了，建议拆分喵～
当前 review 中暴露出的设计问题：
- 数据模型存在冗余且未真正落实 —
binding/rawBinding/hasCustomBinding三元组在当前实现里所有场景下值都相同（已逐场景验证），「区分清空与未设置」的设计意图只停留在字段名上。根源是「清空 vs 禁用」这一基础设计点尚未明确——既然有了enabled，「清空」操作的独立价值很弱 - 占位字段缺乏消费方 —
category每个定义都标了，但代码里没有任何地方读它，意图不明 - 类型边界不收敛 —
supportedPlatforms用NodeJS.Platform[]接受 11 个值但实际只用 3 个 - 命名待商榷 —
persistOnBlur实质就是global - 核心类型缺失 JSDoc —
ShortcutDefinition/ShortcutPreferenceValue是单一真相源，但每个字段都没有 JSDoc - 文档与代码不一致 —
definitions.ts小节的字段说明缺失多个字段
建议的拆分方式：
- PR 1：纯基础设施迁移——把现有快捷键从 Redux/configManager 平迁到 Preference 架构，不引入新的数据模型概念（不要
hasCustomBinding、不要binding/rawBinding分离），保持现状能跑 - PR 2：在 PR 1 之上做数据模型设计，先在 issue 里把「清空 vs 禁用」「category 用途」「平台类型」这些 design question 讨论清楚，再落地
- PR 3：UI 层（设置页录制、冲突检测、分类分组等）
这样每个 PR 的 review 焦点更集中，也避免「设计还在讨论但代码已经写完了」导致后续要么将就要么大改的窘境喵～
具体的 design / docs / suggestion / question 都写在内联 thread 里了 🐱
- Removed serialization logic from shortcuts slice as shortcuts are now managed via PreferenceService. - Updated comments to reflect the migration and the purpose of the shortcuts slice. - Added a new design document detailing the refactor of the shortcut system, including architecture, data flow, and extension guidelines.
…imestamps and shortcut definitions
- Fix Escape key saving as binding instead of canceling recording (C1) - Fix conflict detection using displayKeys instead of normalized binding (C2) - Stabilize useHotkeys callback with useRef to avoid re-registration (W1) - Add guard for ready-to-show callback on stopped service (W3) - Use registerDisposable for preference subscriptions (W4) - Fix antd Table render signature parameter mismatch (S2) - Add duplicate key check in isValidShortcut (S3) Signed-off-by: kangfenmao <[email protected]>
…d enhancing type definitions
- Updated shortcut keys in AppMenuService, ShortcutService, and various components to follow the new structure: 'shortcut.app.core' for core shortcuts and 'shortcut.app' for app-specific shortcuts. - Adjusted preference keys in classification JSON to reflect the new naming convention. - Ensured all related components and services are aligned with the updated shortcut keys for consistency and clarity.
…s and translations - Updated shortcut key mapping from 'toggle_show_assistants' to 'toggle_sidebar' in label.ts. - Changed corresponding translations in en-us.json, zh-cn.json, zh-tw.json, de-de.json, el-gr.json, es-es.json, fr-fr.json, ja-jp.json, pt-pt.json, ro-ro.json, and ru-ru.json. - Modified usage of the shortcut in AgentPage.tsx and HomePage.tsx to reflect the new sidebar toggle logic. - Adjusted migration and initial state in migrate.ts and shortcuts.ts to use 'toggle_sidebar'. - Updated documentation and classification files to align with the new shortcut key.
…e useShortcut options handling
kangfenmao
force-pushed
the
refactor/v2/shortcuts
branch
from
5c40a83 to
f934bd0
Compare
…e shortcut definition
…s and enhance shortcut dependency handling
… and update reset logic
…sed on platform support
…d update shortcut management logic - Changed the key of the shortcut from 'toggle_sidebar' to 'toggle_show_assistants'. - Introduced a new function getSerializableShortcuts to prepare shortcuts for serialization. - Updated the updateShortcut, toggleShortcut, and resetShortcuts reducers to use the new serialization function when updating shortcuts.
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
There was a problem hiding this comment.
Note
This issue/comment/review was translated by Claude.
Overall direction is approved, but the following 4 points need to be addressed before merging.
🚨 Critical
C2. onStop() does not reset isRegisterOnBoot, so first window after hot restart no longer registers boot-state shortcuts
Location: src/main/services/ShortcutService.ts:48-51
The framework supports stop → start to trigger onInit() again, but isRegisterOnBoot is a class field with default value, only initialized once during new; onStop() only does unregisterAll() and mainWindow = null, neither resetting isRegisterOnBoot = true nor handlers.clear(). After service restart, registerForWindow will take the false branch of if (this.isRegisterOnBoot), and the tray startup registration path for ready-to-show will never execute again.
protected async onStop() {
this.unregisterAll()
this.mainWindow = null
this.isRegisterOnBoot = true // +
this.handlers.clear() // + avoid duplicate set after restart registerBuiltInHandlers
}Suggest adding a ShortcutService.test.ts test case to cover shortcuts rebinding after stop → start.
C3. No user feedback for shortcut registration conflicts (occupied by system/other applications)
Location: src/main/services/ShortcutService.ts:238-245
if (success) {
this.registeredAccelerators.set(accelerator, { handler, window: win })
} else {
logger.warn(`Failed to register shortcut ${accelerator}: accelerator is held by another application`)
}globalShortcut.register() returning false is a normal signal from Electron, indicating the accelerator is occupied by the system/other applications (users will frequently encounter this when binding Ctrl+Space, Cmd+Tab combinations). The current implementation only logs a logger.warn, with no red indication/toast on the settings page. Users will see "Save successful" but pressing does nothing—this is the most core user feedback scenario for the shortcut system.
Additionally, findDuplicateLabel in ShortcutSettings.tsx only covers "Cherry Studio internal" binding conflicts, excluding "system-level conflicts".
Suggestions:
- After registration completes, push failed keys to renderer via IPC (can refer to
AppUpdaterService.ts'sIpcChannel.UpdateErrorpattern). ShortcutSettingsdisplays "This shortcut is occupied by system or other application" hint on the corresponding row, with a red border.- The catch on lines 243-244 that actually throws exceptions suggests upgrading to
logger.error(throwing exceptions is a real bug, different from "occupied", should not be same level).
⚠️ Important
I1. window.once('ready-to-show', ...) not registered as disposable
Location: src/main/services/ShortcutService.ts:147-152
window.once('ready-to-show', () => {
if (\!this.mainWindow || this.mainWindow.isDestroyed()) return
if (application.get('PreferenceService').get('app.tray.on_launch')) {
this.registerShortcuts(window, true)
}
})The current guard only defends against window being destroyed. If service has already onStop, but window is still alive (hot restart scenario that only stops ShortcutService without stopping WindowService), this callback will still call this.registerShortcuts—at this point registeredAccelerators and other states have been cleared in unregisterAll, but globalShortcut will register again.
Suggest registering as a disposable:
const onReadyToShow = () => {
if (\!this.mainWindow || this.mainWindow.isDestroyed()) return
if (application.get('PreferenceService').get('app.tray.on_launch')) {
this.registerShortcuts(window, true)
}
}
window.once('ready-to-show', onReadyToShow)
this.registerDisposable(() => window.off('ready-to-show', onReadyToShow))Incidentally addresses the onStop reset issue mentioned in C2.
I2. windowOnHandlers (focus/blur/closed) do not use registerDisposable
Location: src/main/services/ShortcutService.ts:31, 156-169, 260-279
Manually storing listeners with windowOnHandlers Map, manually window.off in onStop, is inconsistent with the lifecycle specification convention that "all resources requiring cleanup should go through registerDisposable". Additionally:
BaseService._doStopcalls_cleanupDisposables()afteronStop()returns—this means ifunregisterAllthrows an error mid-way (e.g.,window.offfails under some edge condition), remaining listeners won't be cleaned up again.- The outer
try...catchwrapping the entireunregisterAll(lines 261-278) swallows unexpected errors, state may stop at an inconsistent point (e.g.,windowOnHandlers.clear()executed butregisteredAccelerators.clear()didn't).
Suggest registering directly in registerForWindow:
if (\!this.windowOnHandlers.has(window)) {
const onFocus = () => this.registerShortcuts(window, false)
const onBlur = () => this.registerShortcuts(window, true)
const onClosed = () => { /* ... */ }
window.on('focus', onFocus)
window.on('blur', onBlur)
window.once('closed', onClosed)
this.windowOnHandlers.set(window, { onFocus, onBlur, onClosed })
this.registerDisposable(() => {
if (window.isDestroyed()) return
window.off('focus', onFocus)
window.off('blur', onBlur)
window.off('closed', onClosed)
})
}unregisterAll should only keep the globalShortcut.unregister part, remove the outer try's broad catch, let unexpected errors bubble to lifecycle manager's unified error path.
Other issues found in the review (antd icons, dead Redux actions, category field has no consumer, test coverage gaps, migration silent skips, etc.) are not being made as mandatory items for this request-changes, can be followed up later.
Original Content
整体方向认可，但以下 4 点需要处理后再合并。
🚨 Critical
C2. onStop() 未重置 isRegisterOnBoot，热重启后首次窗口不再注册启动态 shortcut
位置：src/main/services/ShortcutService.ts:48-51
框架支持 stop → start 再次触发 onInit()，但 isRegisterOnBoot 是类字段默认值，只在 new 时初始化一次；onStop() 只做了 unregisterAll() 和 mainWindow = null，既没复位 isRegisterOnBoot = true，也没 handlers.clear()。service 被重启后，registerForWindow 会走 if (this.isRegisterOnBoot) 的 false 分支，ready-to-show 的 tray 启动注册路径永远不会再执行。
protected async onStop() {
this.unregisterAll()
this.mainWindow = null
this.isRegisterOnBoot = true // +
this.handlers.clear() // + 避免重启后 registerBuiltInHandlers 重复 set
}建议补一个 ShortcutService.test.ts 用例覆盖 stop → start 后 shortcuts 能重新绑定。
C3. 快捷键注册冲突（被系统/其他应用占用）无任何用户反馈
位置：src/main/services/ShortcutService.ts:238-245
if (success) {
this.registeredAccelerators.set(accelerator, { handler, window: win })
} else {
logger.warn(`Failed to register shortcut ${accelerator}: accelerator is held by another application`)
}globalShortcut.register() 返回 false 是 Electron 的常规信号，表示 accelerator 被系统/其他应用占用（用户绑定 Ctrl+Space、Cmd+Tab 等组合时会频繁遇到）。当前实现只打一条 logger.warn，设置页面也没有任何红色提示/toast。用户会看到"保存成功"，但按下去毫无反应——这是 shortcut 系统最核心的用户反馈场景。
此外，ShortcutSettings.tsx 里的 findDuplicateLabel 只覆盖"Cherry Studio 内部"的 binding 冲突，不包含"系统级冲突"。
建议：
- 注册完成后把失败的 key 通过 IPC 推给 renderer（可参考
AppUpdaterService.ts的IpcChannel.UpdateError模式）。 ShortcutSettings在对应行显示 "该快捷键已被系统或其他应用占用" 的提示，并加红色边框。- 第 243-244 行真正抛异常的 catch 建议升级为
logger.error（抛异常是真正的 bug，与"被占用"不同，不应同一 level）。
⚠️ Important
I1. window.once('ready-to-show', ...) 未登记到 disposable
位置：src/main/services/ShortcutService.ts:147-152
window.once('ready-to-show', () => {
if (\!this.mainWindow || this.mainWindow.isDestroyed()) return
if (application.get('PreferenceService').get('app.tray.on_launch')) {
this.registerShortcuts(window, true)
}
})当前 guard 只防御 window 被销毁的情况。若 service 已 onStop，但 window 仍然存活（只停掉 ShortcutService 而不停 WindowService 的热重启场景），这个 callback 仍会调用 this.registerShortcuts——此时 registeredAccelerators 等状态已在 unregisterAll 中被清空，但 globalShortcut 仍会再注册。
建议登记为 disposable：
const onReadyToShow = () => {
if (\!this.mainWindow || this.mainWindow.isDestroyed()) return
if (application.get('PreferenceService').get('app.tray.on_launch')) {
this.registerShortcuts(window, true)
}
}
window.once('ready-to-show', onReadyToShow)
this.registerDisposable(() => window.off('ready-to-show', onReadyToShow))附带解决 C2 中提到的 onStop 复位问题。
I2. windowOnHandlers（focus/blur/closed）未走 registerDisposable
位置：src/main/services/ShortcutService.ts:31, 156-169, 260-279
手动用 windowOnHandlers Map 存 listener、onStop 里手动 window.off，与 lifecycle 规范约定的"所有需要清理的资源统一通过 registerDisposable"不一致。并且：
BaseService._doStop在onStop()返回之后才_cleanupDisposables()——这意味着如果unregisterAll中途抛错（例如window.off在某种边界条件下挂掉），剩余 listener 不会被二次清理。- 外层
try...catch包住整个unregisterAll（第 261-278 行），吞掉非预期错误，状态可能停在不一致点（例如windowOnHandlers.clear()执行了但registeredAccelerators.clear()没执行）。
建议在 registerForWindow 里直接登记：
if (\!this.windowOnHandlers.has(window)) {
const onFocus = () => this.registerShortcuts(window, false)
const onBlur = () => this.registerShortcuts(window, true)
const onClosed = () => { /* ... */ }
window.on('focus', onFocus)
window.on('blur', onBlur)
window.once('closed', onClosed)
this.windowOnHandlers.set(window, { onFocus, onBlur, onClosed })
this.registerDisposable(() => {
if (window.isDestroyed()) return
window.off('focus', onFocus)
window.off('blur', onBlur)
window.off('closed', onClosed)
})
}unregisterAll 里只保留 globalShortcut.unregister 部分，去掉外层 try 的大范围 catch，让非预期错误冒泡到 lifecycle manager 的统一错误路径。
其他 review 中发现的问题（antd icon、dead Redux actions、category 字段无消费方、test 覆盖缺口、迁移静默跳过等），先不作为本次 request-changes 的强制项，可以后续跟进。
…ger and update ShortcutService dependencies
…g and update UI notifications
kangfenmao
force-pushed
the
refactor/v2/shortcuts
branch
from
79dc514 to
e3dd377
Compare
…e conflict notification handling
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
|
@0xfullex Check if there are any parts that need to be modified |
# Conflicts: # src/renderer/src/i18n/translate/de-de.json # src/renderer/src/i18n/translate/el-gr.json # src/renderer/src/i18n/translate/es-es.json # src/renderer/src/i18n/translate/fr-fr.json # src/renderer/src/i18n/translate/ja-jp.json # src/renderer/src/i18n/translate/pt-pt.json # src/renderer/src/i18n/translate/ro-ro.json # src/renderer/src/i18n/translate/ru-ru.json
0xfullex
left a comment
0xfullex
left a comment
There was a problem hiding this comment.
[Request change] labelKey is still hand-duplicated in 3 places
Thanks for the iteration. The ShortcutLabelKey union type was added (good — it catches typos at compile time), but the underlying drift this was meant to fix is still there: the labelKey list now lives in three hand-written places that must stay in sync.
Current state
packages/shared/shortcuts/definitions.ts—SHORTCUT_DEFINITIONS[].labelKey(20 entries, source of truth)packages/shared/shortcuts/types.ts:25-43—ShortcutLabelKeyunion (20 string literals, hand-written)src/renderer/src/i18n/label.ts—shortcutLabelKeyMap: Record<ShortcutLabelKey, string>(20 entries, hand-written)
The Record<ShortcutLabelKey, ...> constraint forces (3) to stay exhaustive w.r.t. (2), which is an improvement. But (2) itself is still a manual copy of the labelKey column from (1), and (3) rewrites the settings.shortcuts.{labelKey} pattern entry-by-entry. Adding a new shortcut requires editing 4 places (definitions + union + map + locale JSON) instead of 2 (definitions + locale JSON).
Suggested fix
Make SHORTCUT_DEFINITIONS the single source of truth by preserving its literal shape and deriving everything else.
Step 1 — definitions.ts: switch from a widening annotation to as const satisfies:
```ts
export const SHORTCUT_DEFINITIONS = [
{ key: 'shortcut.general.show_main_window', scope: 'main', category: 'general', labelKey: 'show_app', global: true },
// ...
] as const satisfies readonly ShortcutDefinition[]
```
Step 2 — types.ts: derive the union from the constant:
```ts
import { SHORTCUT_DEFINITIONS } from './definitions'
export type ShortcutLabelKey = (typeof SHORTCUT_DEFINITIONS)[number]['labelKey']
```
(If the circular-import risk between `types.ts` and `definitions.ts` is a concern, move the derived type into `definitions.ts` or a new `derived.ts`.)
Step 3 — label.ts: derive the map from the constant:
```ts
import { SHORTCUT_DEFINITIONS } from '@shared/shortcuts/definitions'
const shortcutLabelKeyMap = Object.fromEntries(
SHORTCUT_DEFINITIONS.map((d) => [d.labelKey, `settings.shortcuts.${d.labelKey}`])
) as Record<ShortcutLabelKey, string>
```
After
- Adding a new shortcut = add 1 entry to `SHORTCUT_DEFINITIONS` + add 1 i18n entry. That's it.
- No possibility of forgetting `ShortcutLabelKey` or `shortcutLabelKeyMap` — they can't drift, they're derived.
- Runtime assertion for "every `labelKey` actually has an i18n entry" can be added as a unit test if desired, but the derivation already eliminates the common bug class.
This closes the loop on the original [50] comment.
Co-authored-by: fullex <[email protected]>
4 participants
Summary
packages/shared/shortcuts/as single source of truth (definitions, types, utils)ShortcutService(main process) with incrementalglobalShortcutregistration instead ofunregisterAll()useShortcut/useShortcutDisplay/useAllShortcutshooks to read from PreferenceShortcutSettingsUI: real-time key preview during recording, conflict detection with visual hint, search/filterPreferencesMappings.tskey: []) no longer fallback to default bindingv2-refactor-temp/docs/shortcuts/Changes
Shared Layer (
packages/shared/shortcuts/)definitions.ts—SHORTCUT_DEFINITIONSarray as the single source of truth for all 20 shortcutstypes.ts—ShortcutPreferenceKey,ShortcutKey,ShortcutPreferenceValue,ShortcutDefinitionutils.ts—coerceShortcutPreference,convertKeyToAccelerator,formatShortcutDisplay,isValidShortcutMain Process
ShortcutService— incremental globalShortcut registration viaregisteredAcceleratorsSet (diff-based add/remove instead of unregisterAll)configManager.getShortcuts/setShortcuts,IpcChannel.Shortcuts_Update, preload bridgeRenderer
useShortcuts.ts— reads from Preference, supports short key (chat.clear) and full key (shortcut.chat.clear)ShortcutSettings.tsx— real-time key preview, conflict hint ("Already used by XXX"), search filterMigration
rename_topic,toggle_show_topics,edit_last_user_message,select_modelTest plan
pnpm build:checkpasses (280 test files, 4913 tests, 0 failures)