From 6ed6f9d02f5cfb335dcf41f3312992005728eb30 Mon Sep 17 00:00:00 2001 From: zhaoyingzhen Date: Thu, 25 Jun 2026 09:34:54 +0800 Subject: [PATCH] feat(shortcut): support unassigned hotkeys MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Allow a shortcut to be left without any hotkey: it stays visible in the list (shown as "None") without being registered with the compositor, so ReplaceHotkey can intentionally unassign a binding and Reset can restore the default later. - Keep empty-hotkey configs valid and visible without registration - Reset: collect resettable ids, unregister them up front and only mutate local state after the compositor confirms the commit; on commit failure leave the map and dconfig untouched so state stays consistent - Reset: drop reset targets from the map so the async valueChanged restore takes the new-entry path and always emits ShortcutChanged, even when the restored default is unassigned - Add KeyConfig operator==/!= and skip onKeyConfigChanged work (and the spurious ShortcutChanged) when the incoming config is unchanged 支持快捷键不分配任何热键:该项仍显示在列表中(显示为 "None"), 但不向合成器注册,从而 ReplaceHotkey 可以主动取消绑定,Reset 之后 可恢复默认值。 - 空热键配置保持有效并可见,但不注册 - Reset:先收集可重置项并提前注销,仅在合成器确认提交后才修改本地 状态;提交失败时不动 map 与 dconfig,保持状态一致 - Reset:从 map 中移除重置目标,使异步 valueChanged 恢复走新增分支, 即使恢复后的默认值为未分配也能正确发出 ShortcutChanged - 新增 KeyConfig operator==/!=,当传入配置无变化时跳过 onKeyConfigChanged 处理并避免冗余的 ShortcutChanged 信号 Log: support unassigned hotkeys Pms: BUG-365989 Change-Id: I655ae653366dfd6b554002c55e2444a3ee731fa3 --- .../shortcut/src/config/configloader.cpp | 31 ++++-- .../shortcut/src/config/configloader.h | 3 +- .../shortcut/src/core/keybindingmanager.cpp | 97 ++++++++++++++++--- .../shortcut/src/core/keybindingmanager.h | 2 +- .../shortcut/src/core/shortcutconfig.h | 19 +++- .../shortcut/tools/extract_shortcuts_i18n.py | 3 +- .../org.deepin.dde.keybinding_en.ts | 4 + .../org.deepin.dde.keybinding_zh_CN.ts | 4 + 8 files changed, 136 insertions(+), 27 deletions(-) diff --git a/src/plugin-qt/shortcut/src/config/configloader.cpp b/src/plugin-qt/shortcut/src/config/configloader.cpp index fe52bd9..d82e034 100644 --- a/src/plugin-qt/shortcut/src/config/configloader.cpp +++ b/src/plugin-qt/shortcut/src/config/configloader.cpp @@ -80,16 +80,29 @@ void ConfigLoader::reload() // Content changes to existing entries are delivered via DConfig::valueChanged. } -void ConfigLoader::resetConfig() +QStringList ConfigLoader::resettableHotkeyIds() const { - // Reset key hotkeys to defaults. Only keys are touched — gestures have - // no "hotkeys" field. Other key fields (enabled, keyEventFlags...) are - // left alone, and non-modifiable keys (hardware keys, locked entries) - // are skipped. + QStringList ids; + for (const KeyConfig &key : m_keys) { if (!key.modifiable) continue; DConfig *config = m_configs.value(key.subPath); - if (config) config->reset("hotkeys"); + if (config && !config->isDefaultValue("hotkeys")) { + ids.append(key.getId()); + } + } + + return ids; +} + +void ConfigLoader::resetHotkeys(const QStringList &ids) +{ + // Reset key hotkeys to defaults. + for (const QString &id : ids) { + DConfig *config = m_configs.value(id); + if (config) { + config->reset("hotkeys"); + } } } @@ -199,8 +212,10 @@ void ConfigLoader::loadConfig(const QString &subPath, bool newOne) if (isKey) { KeyConfig keyConfig = parseKeyConfig(config); if (!keyConfig.isValid()) { - // isValid() requires enabled && non-empty appId/displayName/hotkeys. - // Most commonly this triggers for shipped-disabled hardware keys + // isValid() requires enabled && non-empty appId/displayName. + // Empty hotkeys are valid: ReplaceHotkey can intentionally leave + // a shortcut unassigned so Reset can restore the default later. + // Most commonly invalid configs are shipped-disabled hardware keys // (e.g. wlan with enabled=false). Remember the subPath so reload() // doesn't re-attempt and log this every dpkg trigger. qWarning() << "Skipping invalid or disabled KeyConfig:" << subPath diff --git a/src/plugin-qt/shortcut/src/config/configloader.h b/src/plugin-qt/shortcut/src/config/configloader.h index 1958105..ae5efc6 100644 --- a/src/plugin-qt/shortcut/src/config/configloader.h +++ b/src/plugin-qt/shortcut/src/config/configloader.h @@ -27,7 +27,8 @@ class ConfigLoader : public QObject void scanForConfigs(); void reload(); - void resetConfig(); + QStringList resettableHotkeyIds() const; + void resetHotkeys(const QStringList &ids); void updateValue(const QString &id, const QString &key, const QVariant &value); void dumpConfigs(); diff --git a/src/plugin-qt/shortcut/src/core/keybindingmanager.cpp b/src/plugin-qt/shortcut/src/core/keybindingmanager.cpp index c5163bb..eef1fb7 100644 --- a/src/plugin-qt/shortcut/src/core/keybindingmanager.cpp +++ b/src/plugin-qt/shortcut/src/core/keybindingmanager.cpp @@ -14,6 +14,13 @@ #include #include +#include + +namespace { +constexpr const char *kDefaultShortcutAppId = "org.deepin.dde.keybinding"; +constexpr const char *kNoHotkeyText = "None"; +} + // Normalize a hotkey from XKB form ("T") to Qt PortableText // ("Ctrl+Alt+T"). Inputs already in Qt form pass through unchanged. // dde-services emits XKB on the wire for legacy control-center compatibility @@ -34,6 +41,12 @@ static QStringList normalizeHotkeys(const QStringList &hotkeys) return out; } +static bool containsEmptyHotkey(const QStringList &hotkeys) +{ + return std::any_of(hotkeys.begin(), hotkeys.end(), + [](const QString &h) { return h.trimmed().isEmpty(); }); +} + KeybindingManager::KeybindingManager(ConfigLoader *loader, ActionExecutor *executor, TranslationManager *translationManager, AbstractKeyHandler *keyHandler, @@ -101,9 +114,7 @@ QList KeybindingManager::ListAllShortcuts() for (const auto &config : m_keyConfigsMap) { // Only expose modifiable shortcuts — control center filters out // non-modifiable entries to avoid showing read-only items. - // Skip shortcuts with no hotkeys (e.g. after ReplaceHotkey stole - // the last binding but before the next reload prunes them). - if (!config.modifiable || config.hotkeys.isEmpty()) { + if (!config.modifiable) { continue; } list.append(toShortcutInfo(config)); @@ -213,6 +224,11 @@ bool KeybindingManager::ModifyHotkeys(const QString &id, const QStringList &newH // Accept either Qt PortableText ("Ctrl+Alt+T") or XKB form // ("T") on the wire; canonicalize to Qt internally. const QStringList normalized = normalizeHotkeys(newHotkeys); + if (normalized.isEmpty() || containsEmptyHotkey(normalized)) { + qWarning() << "ModifyHotkeys: new hotkeys can not be empty:" << id; + return false; + } + // Check for conflicts (exclude self) for (const QString &hotkey : normalized) { ShortcutInfo conflictInfo = LookupConflictShortcut(hotkey); @@ -363,6 +379,10 @@ bool KeybindingManager::ReplaceHotkey(const QString &targetId, const QString &ne } const QString normalized = normalizeHotkey(newHotkey); + if (normalized.trimmed().isEmpty()) { + qWarning() << "ReplaceHotkey: new hotkey can not be empty:" << targetId; + return false; + } // Remove the hotkey from the conflict shortcut if (!conflictConfig.hotkeys.removeOne(normalized)) { @@ -411,14 +431,7 @@ bool KeybindingManager::ReplaceHotkey(const QString &targetId, const QString &ne m_loader->updateValue(targetId, "hotkeys", targetConfig.hotkeys); m_loader->updateValue(conflictId, "hotkeys", conflictConfig.hotkeys); emit ShortcutChanged(targetId, toShortcutInfo(targetConfig)); - if (conflictConfig.hotkeys.isEmpty()) { - // Last hotkey was stolen. - // The shortcut stays in dconfig with empty hotkeys; on next load - // registerShortcut will skip it (isValid requires non-empty hotkeys). - emit ShortcutRemoved(conflictId); - } else { - emit ShortcutChanged(conflictId, toShortcutInfo(conflictConfig)); - } + emit ShortcutChanged(conflictId, toShortcutInfo(conflictConfig)); return true; } @@ -467,7 +480,39 @@ void KeybindingManager::ReloadConfigs() void KeybindingManager::Reset() { // Reset shortcut hotkeys to defaults; other fields (enabled, etc.) are untouched. - m_loader->resetConfig(); + const QStringList resetIds = m_loader->resettableHotkeyIds(); + if (resetIds.isEmpty()) { + return; + } + + // DConfig resets are delivered later as per-key valueChanged signals. + // Release all reset targets first so one restored default does not conflict + // with another target's stale binding. + QStringList toRestore; + for (const QString &id : resetIds) { + if (!m_keyConfigsMap.contains(id)) { + continue; + } + + m_keyHandler->unregisterKey(id); + m_specialKeyHandler->unregisterKey(id); + toRestore.append(id); + } + + if (!m_keyHandler->commitSync()) { + // Compositor rejected the unregister; leave the local map and dconfig + // untouched so state stays consistent with what's actually bound. + qWarning() << "Reset: failed to commit unregistering existing hotkeys"; + return; + } + + // The compositor confirmed the unregister. Drop the local entries so the + // async valueChanged restore goes through onKeyConfigChanged's new-entry path + for (const QString &id : toRestore) { + m_keyConfigsMap.remove(id); + } + + m_loader->resetHotkeys(resetIds); } void KeybindingManager::onKeyConfigChanged(const KeyConfig &config) @@ -485,6 +530,12 @@ void KeybindingManager::onKeyConfigChanged(const KeyConfig &config) } } else { // exist KeyConfig &old = m_keyConfigsMap[config.getId()]; + + // No actual change, skip to avoid spurious ShortcutChanged signal + if (old == config) { + return; + } + if (!config.enabled) { // enable->disable m_keyHandler->unregisterKey(config.getId()); @@ -548,6 +599,12 @@ bool KeybindingManager::registerShortcut(const KeyConfig &config) return false; } + if (config.hotkeys.isEmpty()) { + qInfo() << "Shortcut has no hotkeys, keeping it visible without registration:" + << config.getId(); + return true; + } + if (m_keyConfigsMap.contains(config.getId())) { qWarning() << "Shortcut conflict detected during init: has same appId and displayName" << "hotkeys:" << config.hotkeys @@ -647,14 +704,24 @@ ShortcutInfo KeybindingManager::toShortcutInfo(const KeyConfig &config) // Emit hotkeys in X11/XKB form (e.g. "T", "XF86AudioMute") // so the legacy control-center shortcut page can render each modifier as // a separate chip without doing its own format translation. - info.hotkeys.reserve(config.hotkeys.size()); - for (const QString &hk : config.hotkeys) { - info.hotkeys.append(QKeySequenceConverter::qKeySequenceToXkb(hk)); + if (config.hotkeys.isEmpty()) { + info.hotkeys.append(localizedNoHotkeyText()); + } else { + info.hotkeys.reserve(config.hotkeys.size()); + for (const QString &hk : config.hotkeys) { + info.hotkeys.append(QKeySequenceConverter::qKeySequenceToXkb(hk)); + } } info.localLanguageName = m_translationManager->translate(config.appId, config.displayName); return info; } +QString KeybindingManager::localizedNoHotkeyText() const +{ + return m_translationManager->translate(QString::fromLatin1(kDefaultShortcutAppId), + QString::fromLatin1(kNoHotkeyText)); +} + uint KeybindingManager::GetNumLockState() const { if (m_keyHandler) { diff --git a/src/plugin-qt/shortcut/src/core/keybindingmanager.h b/src/plugin-qt/shortcut/src/core/keybindingmanager.h index cb197ae..60c88bc 100644 --- a/src/plugin-qt/shortcut/src/core/keybindingmanager.h +++ b/src/plugin-qt/shortcut/src/core/keybindingmanager.h @@ -103,6 +103,7 @@ private slots: void rollbackRegistration(const QString &id1, const QString &id2, KeyConfig &config1, KeyConfig &config2, const QStringList &hotkeys1, const QStringList &hotkeys2); + QString localizedNoHotkeyText() const; ConfigLoader *m_loader; @@ -131,4 +132,3 @@ inline const QDBusArgument &operator>>(const QDBusArgument &argument, ShortcutIn return argument; } - diff --git a/src/plugin-qt/shortcut/src/core/shortcutconfig.h b/src/plugin-qt/shortcut/src/core/shortcutconfig.h index 3dcaf26..adfc0a2 100644 --- a/src/plugin-qt/shortcut/src/core/shortcutconfig.h +++ b/src/plugin-qt/shortcut/src/core/shortcutconfig.h @@ -50,7 +50,24 @@ struct KeyConfig : public BaseConfig KeyConfig() : keyEventFlags(KeyEventFlag::Release) {} // Default to release - bool isValid() const { return enabled && !appId.isEmpty() && !displayName.isEmpty() && !hotkeys.isEmpty(); } + bool isValid() const { return enabled && !appId.isEmpty() && !displayName.isEmpty(); } + + bool operator==(const KeyConfig &other) const { + return appId == other.appId && + subPath == other.subPath && + displayName == other.displayName && + category == other.category && + enabled == other.enabled && + modifiable == other.modifiable && + triggerType == other.triggerType && + triggerValue == other.triggerValue && + hotkeys == other.hotkeys && + keyEventFlags == other.keyEventFlags; + } + + bool operator!=(const KeyConfig &other) const { + return !(*this == other); + } }; enum class GestureType { diff --git a/src/plugin-qt/shortcut/tools/extract_shortcuts_i18n.py b/src/plugin-qt/shortcut/tools/extract_shortcuts_i18n.py index 8d09f5d..b35d605 100755 --- a/src/plugin-qt/shortcut/tools/extract_shortcuts_i18n.py +++ b/src/plugin-qt/shortcut/tools/extract_shortcuts_i18n.py @@ -13,7 +13,8 @@ def extract_translations(config_dir, output_file, app_id): Scans the config_dir for org.deepin.shortcut.json / org.deepin.gesture.json files and extracts the displayName field into a dummy C++ file for lupdate scanning. """ - display_names = set() + # Fixed display text used when a shortcut has no assigned key sequence. + display_names = {"None"} target_files = {"org.deepin.shortcut.json", "org.deepin.gesture.json"} for root, dirs, files in os.walk(config_dir): diff --git a/src/plugin-qt/shortcut/translations/org.deepin.dde.keybinding_en.ts b/src/plugin-qt/shortcut/translations/org.deepin.dde.keybinding_en.ts index 1ba968b..197ea08 100644 --- a/src/plugin-qt/shortcut/translations/org.deepin.dde.keybinding_en.ts +++ b/src/plugin-qt/shortcut/translations/org.deepin.dde.keybinding_en.ts @@ -11,5 +11,9 @@ Default terminal + + None + + diff --git a/src/plugin-qt/shortcut/translations/org.deepin.dde.keybinding_zh_CN.ts b/src/plugin-qt/shortcut/translations/org.deepin.dde.keybinding_zh_CN.ts index 9a0f4d1..8add566 100644 --- a/src/plugin-qt/shortcut/translations/org.deepin.dde.keybinding_zh_CN.ts +++ b/src/plugin-qt/shortcut/translations/org.deepin.dde.keybinding_zh_CN.ts @@ -11,5 +11,9 @@ Default terminal 终端 + + None + +