fix(keybinding): resolve shortcut conflicts and improve Wayland support#1088
Open
robertkill wants to merge 1 commit intolinuxdeepin:masterfrom
Open
fix(keybinding): resolve shortcut conflicts and improve Wayland support#1088robertkill wants to merge 1 commit intolinuxdeepin:masterfrom
robertkill wants to merge 1 commit intolinuxdeepin:masterfrom
Conversation
Problem: 1. Shortcut modification via control-center conflicts with dconfig listener 2. System shortcuts not properly synchronized to KWin on Wayland 3. Special shortcuts (screenshotWindow, launcher, systemMonitor) need specific KWin formats Solution: 1. Add modifyingShortcuts set to track API modifications, skip dconfig callback during modification 2. Sync system shortcut changes to KWin immediately when dconfig changes 3. Restore hardcoded accelJson for special shortcuts with KWin-compatible formats 4. Fix single keystroke replacement logic (clear existing except Delete shortcuts) 5. Add enhanced logging for debugging Influence: 1. Test shortcut modification via control-center on Wayland 2. Test system shortcuts (screenshot, launcher, system monitor) on Wayland 3. Test custom shortcut add/delete/clear operations 4. Verify no duplicate shortcut registrations after modification 5. Verify dconfig and KWin stay in sync fix: 修复快捷键冲突和 Wayland 支持问题 问题： 1. 控制中心修改快捷键与 dconfig 监听回调产生竞争冲突 2. Wayland 下系统快捷键未正确同步到 KWin 3. 特殊快捷键（截图窗口、启动器、系统监视器）需要特定 KWin 格式 解决方案： 1. 添加 modifyingShortcuts 集合跟踪 API 修改，修改期间跳过 dconfig 回调 2. dconfig 变更时立即同步系统快捷键到 KWin 3. 恢复特殊快捷键的硬编码 accelJson，使用 KWin 兼容格式 4. 修复单键替换逻辑（清空已有快捷键，Delete 类保留双绑） 5. 添加详细日志便于调试 影响范围： 1. 测试 Wayland 下控制中心修改快捷键 2. 测试 Wayland 下系统快捷键（截图、启动器、系统监视器） 3. 测试自定义快捷键的增删清操作 4. 验证修改后无重复快捷键注册 5. 验证 dconfig 和 KWin 保持同步 PMS: BUG-355747
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: robertkill The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Reviewer's GuideThis PR fixes shortcut conflicts and improves Wayland/KWin integration by adding an API-modification guard, normalizing/special-casing system accelerators for KWin, synchronizing dconfig updates to KWin, tightening single-keystroke replacement behavior, and enhancing logging and diagnostics around keybinding operations. Sequence diagram for API shortcut modification with dconfig guardsequenceDiagram
actor ControlCenter
participant Manager
participant StringSet
participant ShortcutManager
participant DConfig
ControlCenter->>Manager: AddShortcutKeystroke(id, type0, keystroke)
Manager->>StringSet: Add(id)
activate StringSet
StringSet-->>Manager: (ok)
deactivate StringSet
Manager->>ShortcutManager: GetByIdType(id, type0)
ShortcutManager-->>Manager: shortcut
Manager->>ShortcutManager: ModifyShortcutKeystrokes(shortcut, maybeNil)
Manager->>ShortcutManager: AddShortcutKeystroke(shortcut, ksToAdd...)
Manager->>ShortcutManager: SaveKeystrokes via shortcut
Note over DConfig,Manager: DConfig emits change signal after SaveKeystrokes
DConfig-->>Manager: listenDConfigChanged(key=id, type0)
Manager->>StringSet: Has(id)
activate StringSet
StringSet-->>Manager: true
deactivate StringSet
Manager-->>DConfig: skip handling (return)
Manager->>StringSet: Remove(id)
activate StringSet
StringSet-->>Manager: (ok)
deactivate StringSet
ControlCenter-->>Manager: AddShortcutKeystroke returns
Sequence diagram for dconfig-driven system shortcut sync to KWin on WaylandsequenceDiagram
participant DConfig
participant Manager
participant ShortcutManager
participant NormalizeFn
participant KWinWm
DConfig-->>Manager: listenDConfigChanged(key, type0=ShortcutTypeSystem)
Manager->>Manager: check enableListenDConfig
Manager->>Manager: check modifyingShortcuts.Has(key)
alt notBeingModified
Manager->>ShortcutManager: GetByIdType(key, type0)
ShortcutManager-->>Manager: shortcut
Manager->>DConfig: Value(0, key)
DConfig-->>Manager: keystrokesRaw
Manager->>ShortcutManager: ModifyShortcutKeystrokes(shortcut, ParseKeystrokes(keystrokesRaw))
opt Wayland
Manager->>NormalizeFn: NormalizeSystemKeystrokesForKWin(key, keystrokesRaw)
NormalizeFn-->>Manager: normalizedKeystrokes
Manager->>Manager: marshal KWinAccel(Id, normalizedKeystrokes) to accelJson
Manager->>KWinWm: SetAccel(0, accelJson)
KWinWm-->>Manager: ok, err
end
Manager->>Manager: emitShortcutSignal(shortcutSignalChanged, shortcut)
else beingModifiedViaAPI
Manager-->>DConfig: skip (return)
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The hardcoded KWin accelerators for
screenshotWindow,launcher, andsystemMonitorare now duplicated acrossNormalizeSystemKeystrokesForKWin,AddSystemToKwin, andsetAccelForWayland; consider centralizing these special cases in a single helper to avoid divergence or future inconsistencies. - The literal
"<Crtl><Alt>Escape"for the system monitor accelerator appears to contain a typo (CrtlvsCtrl); please double-check this against the actual KWin format to ensure the shortcut is recognized correctly. - Several new
Infoflogs (e.g. inlistenGlobalAccel,setShortForWayland,setAccelForWayland, andDeleteShortcutKeystroke) may be quite verbose in normal operation; consider downgrading high-frequency paths toDebugfor guarding them to avoid noisy logs in production.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The hardcoded KWin accelerators for `screenshotWindow`, `launcher`, and `systemMonitor` are now duplicated across `NormalizeSystemKeystrokesForKWin`, `AddSystemToKwin`, and `setAccelForWayland`; consider centralizing these special cases in a single helper to avoid divergence or future inconsistencies.
- The literal `"<Crtl><Alt>Escape"` for the system monitor accelerator appears to contain a typo (`Crtl` vs `Ctrl`); please double-check this against the actual KWin format to ensure the shortcut is recognized correctly.
- Several new `Infof` logs (e.g. in `listenGlobalAccel`, `setShortForWayland`, `setAccelForWayland`, and `DeleteShortcutKeystroke`) may be quite verbose in normal operation; consider downgrading high-frequency paths to `Debugf` or guarding them to avoid noisy logs in production.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Contributor
|
TAG Bot New tag: 6.1.86 |
Contributor
|
TAG Bot New tag: 6.1.87 |
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
2 participants
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.
Problem:
Solution:
Influence:
fix: 修复快捷键冲突和 Wayland 支持问题
问题：
解决方案：
影响范围：
PMS: BUG-355747
Summary by Sourcery
Resolve shortcut modification conflicts and improve Wayland shortcut synchronization with KWin, including special-case handling for system shortcuts.
Bug Fixes:
Enhancements: