Skip to content

feat(dock): pass launch_type when launching apps from taskbar#1590

Merged
Ivy233 merged 1 commit intolinuxdeepin:masterfrom
Ivy233:feat/launch-type-eventlog
May 8, 2026
Merged

feat(dock): pass launch_type when launching apps from taskbar#1590
Ivy233 merged 1 commit intolinuxdeepin:masterfrom
Ivy233:feat/launch-type-eventlog

Conversation

@Ivy233
Copy link
Copy Markdown
Contributor

@Ivy233 Ivy233 commented May 8, 2026

任务栏启动应用时传入 launch_type 参数,用于应用启动埋点统计。
launch_type=2 表示从任务栏快捷启动。涵盖直接 DBus 调用
和 dde-am CLI 两种启动路径。

Pass launch_type parameter when launching applications from taskbar for application launch event reporting. launch_type=2 indicates the app was launched from the taskbar. Covers both direct DBus calls and dde-am CLI launch paths.

PMS: TASK-388657

Copy link
Copy Markdown

@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.

Sorry @Ivy233, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@Ivy233 Ivy233 force-pushed the feat/launch-type-eventlog branch from f100f03 to 41b2257 Compare May 8, 2026 02:31
@Ivy233 Ivy233 requested a review from BLumia May 8, 2026 05:03
@Ivy233 Ivy233 force-pushed the feat/launch-type-eventlog branch 3 times, most recently from 2b0a626 to a8034b8 Compare May 8, 2026 10:47
@deepin-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

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

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

任务栏启动应用时传入 launch_type 参数,用于应用启动埋点统计。
launch_type=2 表示从任务栏快捷启动。涵盖直接 DBus 调用
和 dde-am CLI 两种启动路径。

Pass launch_type parameter when launching applications from
taskbar for application launch event reporting. launch_type=2
indicates the app was launched from the taskbar. Covers both
direct DBus calls and dde-am CLI launch paths.

PMS: TASK-388657
@Ivy233 Ivy233 force-pushed the feat/launch-type-eventlog branch from a8034b8 to 5f95c98 Compare May 8, 2026 12:18
@deepin-ci-robot
Copy link
Copy Markdown

deepin pr auto review

Git Diff 代码审查报告

总体评价

这段代码主要是为 DDE (Deepin Desktop Environment) 的任务管理器添加了启动类型标识功能,并通过条件编译来支持不同环境。整体逻辑清晰,但有一些可以改进的地方。

详细审查

1. 依赖版本变更 (debian/control)

- dde-application-manager-api (>= 1.2.48),
+ dde-application-manager-api (>> 1.2.51),

问题与建议:

  • 版本号变更从 >= 1.2.48 变为 >> 1.2.51,这是一个严格的版本要求变更
  • >> 表示严格大于,不包含 1.2.51 本身,这可能导致依赖问题
  • 建议确认是否真的需要排除 1.2.51 版本,如果不需要,建议使用 >= 以提高兼容性

2. 代码格式问题 (CMakeLists.txt)

+

 if (BUILD_WITH_X11)

问题与建议:

  • 添加了多余的空行,建议保持代码风格一致性
  • 建议删除这个多余的空行

3. 版权年份更新

-// SPDX-FileCopyrightText: 2023 UnionTech Software Technology Co., Ltd.
+// SPDX-FileCopyrightText: 2023 - 2026 UnionTech Software Technology Co., Ltd.

问题与建议:

  • 版权年份更新到 2026 年,这表示预计维护到 2026 年
  • 建议确认这个年份范围是否合理,通常建议使用当前年份或使用"2023, 2024"这样的格式

4. 启动类型标识添加

m_applicationInterface->Launch(QString(), urls, QVariantMap{{QStringLiteral("_launch_type"), QStringLiteral("dde-shell")}});

问题与建议:

  • 使用 QStringLiteral 是好的做法,可以避免运行时字符串转换
  • 建议将 "dde-shell" 定义为常量,避免在多处硬编码,例如:
    static const QLatin1String LAUNCH_TYPE_DDE_SHELL("dde-shell");
    static const QLatin1String LAUNCH_TYPE_KEY("_launch_type");

5. 条件编译使用

#ifdef HAVE_DDE_API_EVENTLOGGER
    process.start("dde-am", {"--by-user", "--launch-type", "dde-shell", path, action});
#else
    process.start("dde-am", {"--by-user", path, action});
#endif

问题与建议:

  • 条件编译使用得当,支持不同环境
  • 建议考虑将 "dde-am" 和参数列表也定义为常量,提高代码可维护性
  • 建议添加注释说明为什么需要条件编译,以及两种情况的区别

6. 进程启动和等待

process.start("dde-am", {"--by-user", "--launch-type", "dde-shell", path, action});
if (!process.waitForFinished()) {
    qWarning() << "Failed to launch the path:" << path << process.errorString();
    return;
}

问题与建议:

  • waitForFinished() 是阻塞调用,可能会影响 UI 响应
  • 建议考虑使用异步方式处理,例如使用 QProcess::finished 信号
  • 如果必须使用阻塞方式,建议设置合理的超时时间:
    if (!process.waitForFinished(5000)) { // 5秒超时
        qWarning() << "Timeout launching the path:" << path;
        process.kill();
        process.waitForFinished();
        return;
    }

7. 代码重复问题

desktopfileamparser.cppdockglobalelementmodel.cppdockgroupmodel.cpp 中有相似的进程启动代码。

建议:

  • 考虑将这部分逻辑抽取为公共函数或辅助类,减少代码重复
  • 例如创建一个 ProcessLauncher 工具类:
    class ProcessLauncher {
    public:
        static bool launchApplication(const QString &path, const QString &action) {
            QProcess process;
            process.setProcessChannelMode(QProcess::MergedChannels);
    #ifdef HAVE_DDE_API_EVENTLOGGER
            process.start("dde-am", {"--by-user", "--launch-type", "dde-shell", path, action});
    #else
            process.start("dde-am", {"--by-user", path, action});
    #endif
            if (!process.waitForFinished(5000)) {
                qWarning() << "Failed to launch the path:" << path << process.errorString();
                return false;
            }
            return true;
        }
    };

安全性建议

  1. 输入验证pathaction 参数应该进行验证,防止命令注入攻击
  2. 错误处理:增强错误处理,记录更多上下文信息
  3. 资源管理:确保进程资源被正确释放,特别是在错误情况下

性能建议

  1. 异步处理:如前所述,考虑使用异步方式处理进程启动
  2. 常量定义:将字符串常量定义为静态常量,减少内存分配
  3. 进程复用:如果频繁启动进程,考虑进程池或复用机制

总结

这段代码整体质量良好,主要功能是添加启动类型标识。主要改进方向包括:

  1. 消除代码重复
  2. 改进错误处理和资源管理
  3. 考虑使用异步方式处理进程启动
  4. 提高代码可维护性,使用常量代替硬编码字符串
  5. 增强输入验证以提高安全性

@Ivy233 Ivy233 merged commit 07846fc into linuxdeepin:master May 8, 2026
11 of 12 checks passed
@Ivy233 Ivy233 deleted the feat/launch-type-eventlog branch May 8, 2026 13:10
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