Skip to content

fix(shortcuts): display correct group name for each shortcut type#270

Merged
deepin-bot[bot] merged 1 commit into
linuxdeepin:masterfrom
GongHeng2017:202604301553-master-fix
May 9, 2026
Merged

fix(shortcuts): display correct group name for each shortcut type#270
deepin-bot[bot] merged 1 commit into
linuxdeepin:masterfrom
GongHeng2017:202604301553-master-fix

Conversation

@GongHeng2017
Copy link
Copy Markdown
Contributor

Replace hardcoded "Settings" group name with type-specific names (Settings, File, Display, Tools, Edit) based on ShortCutType enum.

修复快捷键分组名称硬编码为"Settings"的bug,根据ShortCutType枚举
正确显示对应的分组名称。

Log: 修复快捷键分组名称显示错误
Bug: https://pms.uniontech.com/bug-view-354843.html
Influence: 快捷键列表中各分组将显示正确的名称,不再统一显示为"Settings"。

Replace hardcoded "Settings" group name with type-specific names
(Settings, File, Display, Tools, Edit) based on ShortCutType enum.

修复快捷键分组名称硬编码为"Settings"的bug,根据ShortCutType枚举
正确显示对应的分组名称。

Log: 修复快捷键分组名称显示错误
Bug: https://pms.uniontech.com/bug-view-354843.html
Influence: 快捷键列表中各分组将显示正确的名称,不再统一显示为"Settings"。
@deepin-ci-robot
Copy link
Copy Markdown

deepin pr auto review

这段代码的修改主要是将快捷键组的名称从硬编码的字符串改为根据 ShortCutType 枚举值动态获取。以下是对这段代码的详细审查意见,包括语法逻辑、代码质量、性能和安全方面的改进建议:

1. 语法逻辑与代码结构

现状

  • 代码使用了 switch-case 结构将枚举 ShortCutType 映射为本地化字符串(tr())。
  • 逻辑上没有明显错误,能够正确根据类型生成对应的组名。

改进建议

  • 增加 default 分支:虽然 switch 覆盖了已知的 5 种类型,但为了防止未来扩展枚举时遗漏处理,建议添加 default 分支,或者使用 Q_UNREACHABLE() 宏(如果所有分支必须处理)。
  • 提取映射逻辑:如果这段映射逻辑在其他地方也会用到,建议将其提取为一个独立的静态辅助函数或静态 QMap/QHash,避免重复代码。

2. 代码质量

现状

  • 变量命名 strType 略显模糊,建议改为更具描述性的名称,如 groupDisplayName
  • tr() 的使用符合 Qt 的国际化规范,值得肯定。

改进建议

  • 使用 Q_ENUM 和元对象系统:如果 ShortCutType 是一个 Qt 枚举,可以使用 Q_ENUM 宏配合 QMetaEnum 来实现枚举到字符串的转换,这样可以减少手动维护 switch-case 的成本,且更符合 Qt 的惯用法。
  • 代码复用:如果这种映射关系在多处使用,建议封装成一个函数,例如:
    QString ShortCutShow::groupDisplayName(ShortCutType type) {
        switch (type) {
            case ShortCutType::Settings: return tr("Settings");
            case ShortCutType::File: return tr("File");
            // ... 其他分支
            default: return tr("Unknown");
        }
    }

3. 性能

现状

  • 每次调用 show() 时都会执行 switch-casetr() 调用,这在性能上没有明显问题,因为 tr() 的开销很小,且快捷键组的数量通常很少。

改进建议

  • 如果 show() 被频繁调用(例如在 UI 刷新时),可以缓存这些组名,避免重复计算。例如,可以在类初始化时构建一个静态 QHash<ShortCutType, QString> 来存储映射关系。

4. 安全性

现状

  • 代码没有明显的安全问题,因为 ShortCutType 是枚举,且 tr() 返回的是本地化字符串,不会导致注入风险。

改进建议

  • 如果 ShortCutType 的值可能来自外部输入(例如从文件或网络读取),则需要验证其有效性,避免未定义行为。但在此场景下,ShortCutType 应该是内部控制的,因此风险较低。

5. 其他建议

  • 国际化上下文:如果 tr() 的翻译上下文(context)需要更明确,可以指定翻译上下文,例如:
    strType = tr("ShortCutShow", "Settings"); // "ShortCutShow" 是上下文
  • 代码可读性:如果 ShortCutType 的枚举值和显示名称直接对应(例如枚举值本身就是可读的字符串),可以考虑直接使用枚举名作为默认值,减少手动映射。

改进后的代码示例

void ShortCutShow::show() {
    // ... 其他代码
    for (ShortCutType type : listType) {
        QJsonObject group;
        QString groupDisplayName = getGroupDisplayName(type); // 提取映射逻辑
        group.insert("groupName", groupDisplayName);

        QJsonArray items;
        for (const auto &d : m_shortcutMap[type]) {
            QJsonObject jsonItem;
            jsonItem.insert("name", d.second);
            // ... 其他代码
        }
    }
}

QString ShortCutShow::getGroupDisplayName(ShortCutType type) {
    switch (type) {
        case ShortCutType::Settings: return tr("Settings");
        case ShortCutType::File: return tr("File");
        case ShortCutType::Display: return tr("Display");
        case ShortCutType::Tools: return tr("Tools");
        case ShortCutType::Edit: return tr("Edit");
        default: return tr("Unknown"); // 防御性编程
    }
}

总结

这段代码的修改是合理的,但可以通过提取映射逻辑、增加 default 分支、优化命名等方式进一步提升代码质量和可维护性。如果性能是关键点,可以考虑缓存映射结果。

@deepin-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: GongHeng2017, lzwind

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@GongHeng2017
Copy link
Copy Markdown
Contributor Author

/merge

@deepin-bot deepin-bot Bot merged commit c017768 into linuxdeepin:master May 9, 2026
10 checks passed
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.

3 participants