Skip to content

Conversation

@BLumia
Copy link
Member

@BLumia BLumia commented Sep 22, 2025

任务栏右键菜单,应用自己的 action 未显示成当前用户的系统语言对应的文案

Summary by Sourcery

Update AMAppItem localization logic to ensure desktop menu actions and icons display in the current user's locale

Bug Fixes:

  • Fix desktop menu actions showing English instead of the user's locale language
  • Correct action name retrieval to use the localeCode rather than the action key

Enhancements:

  • Enhance getLocaleOrDefaultValue to first check full locale code, then language prefix, before falling back
  • Simplify icon name lookup by using a direct map value instead of locale-based fallback

任务栏右键菜单,应用自己的 action 未显示成当前用户的系统语言对应的文案

PMS: BUG-333955
Log:
@deepin-ci-robot
Copy link

deepin pr auto review

我对这段代码进行了仔细审查,以下是我的分析和改进建议:

1. 语法逻辑

问题:

  • getLocaleOrDefaultValue 函数的参数命名不够清晰,targetKey 被改为 localeCode 后,函数内部逻辑与参数名不完全匹配。
  • onPropertyChanged 函数中,直接使用 Application::icons().value(DESKTOP_ENTRY_ICON_KEY) 而不通过 getLocaleOrDefaultValue,这可能导致不一致的行为。

改进建议:

  • 统一使用 getLocaleOrDefaultValue 处理图标名称,保持逻辑一致性。
  • 考虑将 localeCode 参数重命名为更准确的名称,如 preferredLocale

2. 代码质量

问题:

  • getLocaleOrDefaultValue 函数中的字符串处理逻辑较为复杂,可读性有待提高。
  • 缺少对输入参数的有效性检查。

改进建议:

  • getLocaleOrDefaultValue 中的字符串处理逻辑拆分为更小的函数,提高可读性。
  • 添加参数有效性检查,确保 valuefallbackKey 不为空。

示例改进:

QString AMAppItem::getLocaleOrDefaultValue(const QStringMap &value, const QString &preferredLocale, const QString &fallbackKey)
{
    Q_ASSERT_X(!value.isEmpty(), "getLocaleOrDefaultValue", "Value map cannot be empty");
    Q_ASSERT_X(!fallbackKey.isEmpty(), "getLocaleOrDefaultValue", "Fallback key cannot be empty");
    
    if (value.contains(preferredLocale)) {
        return value.value(preferredLocale);
    }
    
    // 处理带区域设置的locale(如zh_CN)
    if (preferredLocale.contains('_')) {
        QString baseLocale = preferredLocale.split('_').first();
        if (value.contains(baseLocale)) {
            return value.value(baseLocale);
        }
    }
    
    return value.value(fallbackKey);
}

3. 代码性能

问题:

  • 字符串操作(如 split)在频繁调用时可能影响性能。
  • 每次调用都会进行字符串检查和处理,没有缓存机制。

改进建议:

  • 考虑使用 QLocale 类来处理本地化相关的逻辑,它可能提供更好的性能。
  • 对于频繁调用的函数,可以考虑添加缓存机制。

4. 代码安全

问题:

  • 字符串操作没有进行边界检查,可能导致越界访问。
  • 缺少对输入参数的验证。

改进建议:

  • 在字符串操作前添加边界检查。
  • 使用 QString::split 时确保不会因为空字符串导致问题。
  • 添加参数验证,确保输入的有效性。

示例改进:

QString AMAppItem::getLocaleOrDefaultValue(const QStringMap &value, const QString &preferredLocale, const QString &fallbackKey)
{
    // 参数验证
    if (value.isEmpty() || fallbackKey.isEmpty()) {
        return QString();
    }
    
    // 检查首选locale是否存在
    if (value.contains(preferredLocale)) {
        return value.value(preferredLocale);
    }
    
    // 安全地处理带区域设置的locale
    if (!preferredLocale.isEmpty() && preferredLocale.contains('_')) {
        QStringList localeParts = preferredLocale.split('_', Qt::SkipEmptyParts);
        if (!localeParts.isEmpty()) {
            QString baseLocale = localeParts.first();
            if (value.contains(baseLocale)) {
                return value.value(baseLocale);
            }
        }
    }
    
    // 返回fallback值
    return value.value(fallbackKey);
}

5. 其他建议

  1. 考虑使用枚举或常量定义来替代魔术字符串(如 DEFAULT_KEY)。
  2. 添加适当的注释说明函数的行为和参数含义。
  3. 考虑使用单元测试来确保 getLocaleOrDefaultValue 的各种场景都能正确处理。

这些改进将有助于提高代码的可维护性、可读性和健壮性。

@sourcery-ai
Copy link

sourcery-ai bot commented Sep 22, 2025

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Enhance locale resolution for desktop menu actions and application icons by refactoring utility logic, updating method parameters, and simplifying lookups to ensure UI labels match the user’s current language settings.

Class diagram for updated AMAppItem locale resolution methods

classDiagram
    class AMAppItem {
        +void setOnDesktop(bool on)
        -QString getLocaleOrDefaultValue(const QStringMap &value, const QString &localeCode, const QString &fallbackKey)
        -void updateActions(const QStringList &actions, const PropMap &actionName)
    }
    AMAppItem --|> Application
    AMAppItem --|> AppItem
Loading

Flow diagram for locale-based action name resolution

flowchart TD
    A["User triggers desktop menu action"] --> B["updateActions() called"]
    B --> C["getLocaleOrDefaultValue() called with locale code"]
    C --> D{Does value contain localeCode?}
    D -- Yes --> E["Return value for localeCode"]
    D -- No --> F{Does localeCode contain '_'?}
    F -- Yes --> G["Extract prefix and check value for prefix"]
    G -- Found --> H["Return value for prefix"]
    G -- Not found --> I["Return fallback value"]
    F -- No --> I["Return fallback value"]
Loading

File-Level Changes

Change Details Files
Refactor locale resolution utility for generic locale codes with prefix fallback
  • Renamed function parameter from targetKey to localeCode
  • Added explicit lookup for full localeCode and prefix fallback when underscore present
  • Return fallback value if no matching locale entry
applets/dde-apps/amappitem.cpp
applets/dde-apps/amappitem.h
Simplify application icon name retrieval
  • Replaced getLocaleOrDefaultValue on Application::icons() with direct icons().value lookup
applets/dde-apps/amappitem.cpp
Fix action name localization to use actual locale code
  • Updated getLocaleOrDefaultValue call in updateActions to pass locale variable instead of action identifier
applets/dde-apps/amappitem.cpp

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey there - I've reviewed your changes and they look great!


Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: 18202781743, BLumia

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

@BLumia BLumia merged commit d93d7bd into linuxdeepin:master Sep 22, 2025
8 of 11 checks passed
@BLumia BLumia deleted the pms-333955 branch September 22, 2025 11:23
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