feat: add category-based skip list for cgroups grouping #1384
+102
−10
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Ivy233 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅ |
Reviewer's GuideAdds a configurable, category-based skip list to the task manager’s cgroups-based window grouping, so that apps in specified desktop Categories (default: TerminalEmulator) bypass cgroup-based grouping and avoid being incorrectly grouped under terminal emulators. Sequence diagram for cgroups-based grouping with category skip checksequenceDiagram
actor User
participant TerminalEmulator
participant GUIApp
participant TaskManager
participant Settings
participant ApplicationManager
participant Application
User->>TerminalEmulator: Start terminal emulator
User->>TerminalEmulator: Run graphical app command
TerminalEmulator->>GUIApp: Spawn GUI process
GUIApp->>TaskManager: New window detected
TaskManager->>Settings: cgroupsBasedGrouping()
alt cgroups based grouping enabled
TaskManager->>TaskManager: getDesktopIdByPid(identifies)
alt desktopId is empty
TaskManager->>TaskManager: Fallback grouping (no desktopId)
else desktopId is not empty
TaskManager->>Settings: cgroupsBasedGroupingSkipIds()
alt desktopId in skip id list
TaskManager->>TaskManager: Skip cgroups grouping (whitelisted by id)
else desktopId not in skip id list
TaskManager->>TaskManager: shouldSkipCgroupsByCategories(desktopId)
TaskManager->>Settings: cgroupsBasedGroupingSkipCategories()
TaskManager->>ApplicationManager: Create Application proxy with desktopId
alt Application proxy is valid
TaskManager->>Application: categories()
alt category in skip category list
TaskManager->>TaskManager: Skip cgroups grouping (whitelisted by category)
else no matching category
TaskManager->>TaskManager: Perform cgroups based grouping
end
else Application proxy invalid
TaskManager->>TaskManager: Perform cgroups based grouping
end
end
end
else cgroups based grouping disabled
TaskManager->>TaskManager: Use existing non cgroups grouping
end
TaskManager-->>User: Window appears in taskbar (grouped or independent depending on checks)
Class diagram for updated TaskManagerSettings and Application interface usageclassDiagram
class TaskManagerSettings {
-bool m_cgroupsBasedGrouping
-QStringList m_dockedElements
-QStringList m_cgroupsBasedGroupingSkipAppIds
-QStringList m_cgroupsBasedGroupingSkipCategories
+TaskManagerSettings(QObject parent)
+bool cgroupsBasedGrouping()
+QStringList cgroupsBasedGroupingSkipIds()
+QStringList cgroupsBasedGroupingSkipCategories()
+QStringList dockedElements()
+void setDockedElements(QStringList elements)
+void toggleDockedElement(QString element)
}
class TaskManager {
+bool init()
}
class Application {
+QStringList categories()
+bool isValid()
}
class Settings {
+bool cgroupsBasedGrouping()
+QStringList cgroupsBasedGroupingSkipIds()
+QStringList cgroupsBasedGroupingSkipCategories()
}
TaskManager --> Settings : uses
TaskManager --> Application : queries categories via DBus
Settings --> TaskManagerSettings : wraps persistent config
TaskManagerSettings ..> QStringList : stores skip lists
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The per-window DBus lookup in shouldSkipCgroupsByCategories may add noticeable overhead in the hot path of window matching; consider caching categories by desktopId or precomputing the skip decision to avoid repeated DBus calls.
- When checking skipCategories.contains(category), you might want to normalize case or use a QSet for skipCategories to avoid subtle mismatches and to make the membership checks more efficient.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The per-window DBus lookup in shouldSkipCgroupsByCategories may add noticeable overhead in the hot path of window matching; consider caching categories by desktopId or precomputing the skip decision to avoid repeated DBus calls.
- When checking skipCategories.contains(category), you might want to normalize case or use a QSet for skipCategories to avoid subtle mismatches and to make the membership checks more efficient.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
BLumia
reviewed
Jan 4, 2026
Comment on lines
80
to
90
| QString dbusPath = QStringLiteral("/org/desktopspec/ApplicationManager1/") + escapeToObjectPath(desktopId); | ||
|
|
||
| // 创建 Application 接口 | ||
| Application appInterface(QStringLiteral("org.desktopspec.ApplicationManager1"), | ||
| dbusPath, | ||
| QDBusConnection::sessionBus()); | ||
|
|
||
| if (!appInterface.isValid()) { | ||
| qCDebug(taskManagerLog) << "Failed to create Application interface for:" << desktopId; | ||
| return false; | ||
| } |
Member
There was a problem hiding this comment.
不再绕一次 dbus 了，在 AppItem 一层新增一个 CategoryRole（可以参考现有的 DDECategoryRole），然后直接使用这个新增的 role 即可。
Contributor
Author
|
recheck |
Ivy233
force-pushed
the
feature/cgroups-skip-categories
branch
from
a0e2e66 to
3b87e6d
Compare
BLumia
reviewed
Jan 7, 2026
| if (activeAppModel) { | ||
| auto existingItem = activeAppModel->match(activeAppModel->index(0, 0), roleNames.key("desktopId"), desktopId, 1, Qt::MatchFixedString | Qt::MatchWrap).value(0); | ||
| if (existingItem.isValid()) { | ||
| categories = activeAppModel->data(existingItem, roleNames.key("categories")).toStringList(); |
Member
There was a problem hiding this comment.
不用再从 roleNames.key("categories") 获取 role 了吧？已经有 TaskManager::CategoriesRole 了。
BLumia
reviewed
Jan 7, 2026
| } | ||
|
|
||
| // 检查是否有任何 category 在跳过列表中 | ||
| for (const QString &category : categories) { |
Member
There was a problem hiding this comment.
是不是反了，不应该是迭代 skipCategories 然后判断 categories 是否包含 skipCategoriy 吗？
skipCategories 配置里预期不会有太多值，默认只有终端，也不期望用户自己随便加。
BLumia
reviewed
Jan 7, 2026
| } | ||
|
|
||
| // 检查应用的 Categories 是否在跳过列表中 | ||
| static bool shouldSkipCgroupsByCategories(const QString &desktopId, QAbstractItemModel *activeAppModel, const QHash<int, QByteArray> &roleNames) |
Member
There was a problem hiding this comment.
倒不如直接把 “尝试通过AM(Application Manager)匹配应用程序” 那块提出来。只提目前这一部分有点怪怪的。
|
TAG Bot New tag: 2.0.25 |
Contributor
Author
|
recheck |
|
TAG Bot New tag: 2.0.26 |
Ivy233
force-pushed
the
feature/cgroups-skip-categories
branch
from
3b87e6d to
62a5b07
Compare
1. 新增 DConfig 配置项 cgroupsBasedGroupingSkipCategories 2. 默认跳过 TerminalEmulator 类别的应用，避免终端中启动的程序被错误识别 3. 在 dde-apps 的 AppItem 中添加 categories 属性 4. 在 AppItemModel 中添加 CategoriesRole 5. 在 AMAppItem 构造函数中保存从 AM 获取的 categories 6. 实现 shouldSkipCgroupsByCategories() 函数，从 dde-apps 缓存读取 categories 7. 提取 tryMatchByApplicationManager() 函数，优化代码结构 8. 使用 TaskManager::CategoriesRole 枚举常量替代运行时查找 9. 优化迭代逻辑，迭代 skipCategories 而非 categories，提升性能 10. 在窗口匹配流程中集成类别检查逻辑 Log: 新增基于应用类别的 cgroups 分组跳过列表，解决终端中启动的图形程序被错误归类的问题 Influence: 1. 测试在 Alacritty 等终端模拟器中启动图形应用 2. 验证新启动的应用在任务栏显示独立图标 3. 验证 categories 从 dde-apps 缓存读取，无额外 DBus 调用 4. 检查 DConfig 配置项可正常读取和修改 5. 确认不影响非终端类应用的正常分组行为 6. 验证深度终端等原有白名单应用仍正常工作 7. 确认代码结构优化不影响功能正确性 PMS: TASK-384865
Ivy233
force-pushed
the
feature/cgroups-skip-categories
branch
from
62a5b07 to
6fe519b
Compare
deepin pr auto review代码审查报告总体概述这段代码实现了一个新功能，允许基于应用程序的类别(Categories)来决定是否跳过基于cgroups的任务图标分组。主要修改包括添加了新的配置项、模型角色和相关检查逻辑。 详细审查1. 语法与逻辑优点
问题与建议
2. 代码质量优点
问题与建议
3. 代码性能问题与建议
4. 代码安全问题与建议
总结这段代码实现了一个有用的功能，整体质量良好。主要改进方向包括：
建议在合并前进行充分测试，特别是测试各种边界情况和异常输入。 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
概述
新增基于应用类别（Categories）的 cgroups 分组跳过列表功能，解决终端模拟器中启动的图形程序被错误归类到终端应用的问题。
问题描述
目前任务栏基于 cgroup 识别应用时，终端中启动的图形程序会继承父进程（终端）的 cgroup 信息，导致窗口被错误识别为归属终端应用。虽然存在白名单 cgroupsBasedGroupingSkipAppIds，但需要逐个手动添加终端模拟器的 appId。
解决方案
新增 DConfig 配置项 cgroupsBasedGroupingSkipCategories
实现类别检查函数 shouldSkipCgroupsByCategories()
集成到窗口匹配流程
修改文件
测试验证
Summary by Sourcery
Add configurable category-based skip list for cgroup-based taskbar grouping to avoid mis-grouping terminal-launched GUI apps.
New Features:
Enhancements: