Skip to content

Conversation

@BLumia
Copy link
Member

@BLumia BLumia commented Oct 31, 2025

增加选项以供对于 cgroup 的应用识别场景,跳过指定应用程序的检查.

由于用户从终端执行带图形界面的应用程序的概率较大(例如 gitk, deepin-dconfig-edtor 等),这些应用必然无法匹配到一个 desktop id, 导致在经过 AM 的应用识别时,会根据其 cgroup 匹配到父进程的应用 id 上. 若要保留基于 cgroup 的识别则此问题基本没有合理的解决方案.

此提交引入了一个新的 dconfig 配置项,允许指定一个列表,表示希望跳过基于 cgroup 的应用识别与合并的 appid 列表. 当 fallback 到 cgroup 识别且 cgroup 在此列表中,则此应用不再根据 cgroup 提取到的 appid 进行分组.

这个列表的值是 appid, 即 desktop-id 去掉 .desktop 后缀后剩余的部分. 此列表一般应当是终端模拟器的应用 id, 也可以将专门用于启动其他程序的第三方启动器/工具添加到此列表中.例如 uTools 或者文件管理器.

另外此提交也把原本允许彻底禁用基于 cgroups 识别与分组的选项的逻辑添加了回来.

另注:

  1. 对于希望在 deepin 环境尽可能保持"deepin原生"行为的组件,应当 接入 dde-application-manager 来通过它启动应用
  2. 所有应用仍然应当尽可能被正确安装到系统中,以确保其存在对应的 desktop-id/appid. 上述新增的配置项也仅适用于被正确安装/有应 用id的应用程序

Summary by Sourcery

Introduce a new skip-appids configuration to refine cgroup-based application detection and re-enable the existing option to disable cgroup grouping entirely

Enhancements:

  • Add configuration setting to exclude specified desktop applications from cgroups-based appid grouping
  • Restore the ability to completely disable cgroups-based grouping via a dconfig option

Documentation:

  • Update dconfig schema to include the new skip-appids setting

增加选项以供对于 cgroup 的应用识别场景,跳过指定应用程序的检查.

由于用户从终端执行带图形界面的应用程序的概率较大(例如 gitk,
deepin-dconfig-edtor 等),这些应用必然无法匹配到一个 desktop id,
导致在经过 AM 的应用识别时,会根据其 cgroup 匹配到父进程的应用
id 上. 若要保留基于 cgroup 的识别则此问题基本没有合理的解决方案.

此提交引入了一个新的 dconfig 配置项,允许指定一个列表,表示希望跳
过基于 cgroup 的应用识别与合并的 appid 列表. 当 fallback 到 cgroup
识别且 cgroup 在此列表中,则此应用不再根据 cgroup 提取到的 appid
进行分组.

这个列表的值是 appid, 即 desktop-id 去掉 .desktop 后缀后剩余的部分.
此列表一般应当是终端模拟器的应用 id, 也可以将专门用于启动其他程
序的第三方启动器/工具添加到此列表中.例如 uTools 或者文件管理器.

另外此提交也把原本允许彻底禁用基于 cgroups 识别与分组的选项的逻
辑添加了回来.

另注:

1. 对于希望在 deepin 环境尽可能保持"deepin原生"行为的组件,应当
   接入 dde-application-manager 来通过它启动应用
2. 所有应用仍然应当尽可能被正确安装到系统中,以确保其存在对应的
   desktop-id/appid. 上述新增的配置项也仅适用于被正确安装/有应
   用id的应用程序

Log:
@BLumia BLumia requested review from 18202781743 and wjyrich October 31, 2025 09:06
@deepin-ci-robot
Copy link

deepin pr auto review

我来对这个diff进行代码审查:

  1. 代码逻辑审查:
  • 新增的cgroupsBasedGroupingSkipAppIds配置项逻辑合理,允许指定某些应用跳过cgroups分组
  • 在taskmanager.cpp中,对分组逻辑的判断条件正确,先检查是否启用cgroups分组,再检查是否在跳过列表中
  • 配置项的默认值设置合理(deepin-terminal)
  1. 代码质量审查:
  • 命名规范:
    • 配置项名称"cgroupsBasedGroupingSkipAppIds"符合驼峰命名规范
    • 函数名"cgroupsBasedGroupingSkipIds()"命名清晰,但与配置项名称略有差异,建议保持一致性
  • 代码结构:
    • 新增的配置项声明、初始化和访问方法都放在了合适的位置
    • 配置项的国际化支持完整
  1. 性能优化建议:
  • QStringList的contains操作在大量数据时可能较慢,建议考虑使用QSet来存储跳过的应用ID,可以提高查找效率:
    QSet<QString> m_cgroupsBasedGroupingSkipAppIds;
  1. 安全性审查:
  • 配置项的权限设置为"readwrite"是合适的,允许用户修改跳过列表
  • visibility设置为"private"是合理的,这是内部实现细节
  1. 其他改进建议:
  • 在taskmanager.cpp中的日志输出可以增加更多调试信息,比如打印跳过分组的原因
  • 建议在配置项变更时添加信号通知,以便其他组件可以响应变化:
    void cgroupsBasedGroupingSkipIdsChanged();
  • 配置项的描述文本可以更详细一些,说明为什么某些应用需要跳过分组
  1. 文档完整性:
  • 配置项的国际化描述完整,包含了中英文
  • 建议在代码注释中添加更多关于cgroups分组和跳过机制的技术说明

总结:
这个改动整体上是合理的,实现了允许特定应用跳过cgroups分组的功能。主要需要关注性能优化和文档完善方面的改进。建议在后续版本中考虑使用QSet来优化查找性能,并添加配置变更通知机制。

@sourcery-ai
Copy link

sourcery-ai bot commented Oct 31, 2025

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

This PR refines cgroup-based application detection by reintroducing the global toggle for grouping, adding a user-defined skip-list of app IDs, and updating the TaskManager logic and configuration schema to honor these new settings.

Sequence diagram for cgroup-based appid detection with skip-list

sequenceDiagram
participant "TaskManager"
participant "Settings"
participant "Model"

"TaskManager"->>"Settings": cgroupsBasedGrouping()
alt cgroupsBasedGrouping enabled
    "TaskManager"->>"Settings": cgroupsBasedGroupingSkipIds()
    "TaskManager"->>"TaskManager": getDesktopIdByPid(identifies)
    alt desktopId not empty and not in skip-list
        "TaskManager"->>"Model": match(desktopId)
        alt match found
            "TaskManager"->>"TaskManager": return result
        end
    end
end
Loading

Entity relationship diagram for new dconfig key and appid skip-list

erDiagram
    TASKMANAGER_DCONFIG {
        string cgroupsBasedGrouping
        string cgroupsBasedGroupingSkipAppIds
        string dockedElements
    }
    TASKMANAGER_DCONFIG ||--o| APP : contains
    APP {
        string appid
    }
    TASKMANAGER_DCONFIG ||--|{ APP : skip-list cgroupsBasedGroupingSkipAppIds
Loading

Class diagram for updated TaskManagerSettings structure

classDiagram
class TaskManagerSettings {
    - bool m_windowSplit
    - bool m_cgroupsBasedGrouping
    - QStringList m_dockedElements
    - QStringList m_cgroupsBasedGroupingSkipAppIds
    + bool cgroupsBasedGrouping() const
    + QStringList cgroupsBasedGroupingSkipIds() const
    + void setWindowSplit(bool split)
    + void setDockedElements(const QStringList &elements)
    + void toggleDockedElement(const QString &element)
}
Loading

File-Level Changes

Change Details Files
Conditional cgroup-based grouping in TaskManager logic
  • Wrap AM desktop ID matching inside the cgroupsBasedGrouping flag check
  • Add check to skip matching when desktopId is in the skip-list
panels/dock/taskmanager/taskmanager.cpp
Expose skip-list setting for cgroup grouping
  • Initialize m_cgroupsBasedGroupingSkipAppIds from dconf with a default entry
  • Provide cgroupsBasedGroupingSkipIds() getter
panels/dock/taskmanager/taskmanagersettings.cpp
panels/dock/taskmanager/taskmanagersettings.h
Define new config key for skip-list
  • Add TASKMANAGER_CGROUPS_BASED_GROUPING_SKIP_APPIDS constant to globals
panels/dock/taskmanager/globals.h
Update dconf schema to include skip-list option
  • Introduce skip-list property in org.deepin.ds.dock.taskmanager.json
panels/dock/taskmanager/dconfig/org.deepin.ds.dock.taskmanager.json

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 - here's some feedback:

  • Register the new 'cgroupsBasedGroupingSkipAppIds' key in the dconf schema JSON with its default value to ensure it's recognized by the settings framework.
  • Consider adding a debug log when an app ID is skipped due to being in the skip list to improve observability of grouping behavior.
  • Normalize or validate the casing of desktop IDs in the skip list to prevent case-sensitivity mismatches during matching.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- Register the new 'cgroupsBasedGroupingSkipAppIds' key in the dconf schema JSON with its default value to ensure it's recognized by the settings framework.
- Consider adding a debug log when an app ID is skipped due to being in the skip list to improve observability of grouping behavior.
- Normalize or validate the casing of desktop IDs in the skip list to prevent case-sensitivity mismatches during matching.

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 802e7f6 into linuxdeepin:master Oct 31, 2025
10 of 11 checks passed
@BLumia BLumia deleted the cgroup-skip-appids branch October 31, 2025 09:57
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