feat: add category-based skip list for cgroups grouping#1384
feat: add category-based skip list for cgroups grouping#1384BLumia merged 1 commit intolinuxdeepin:masterfrom
Conversation
|
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
|
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.
| 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; | ||
| } |
There was a problem hiding this comment.
不再绕一次 dbus 了,在 AppItem 一层新增一个 CategoryRole(可以参考现有的 DDECategoryRole),然后直接使用这个新增的 role 即可。
|
recheck |
a0e2e66 to
3b87e6d
Compare
| 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(); |
There was a problem hiding this comment.
不用再从 roleNames.key("categories") 获取 role 了吧?已经有 TaskManager::CategoriesRole 了。
| } | ||
|
|
||
| // 检查是否有任何 category 在跳过列表中 | ||
| for (const QString &category : categories) { |
There was a problem hiding this comment.
是不是反了,不应该是迭代 skipCategories 然后判断 categories 是否包含 skipCategoriy 吗?
skipCategories 配置里预期不会有太多值,默认只有终端,也不期望用户自己随便加。
| } | ||
|
|
||
| // 检查应用的 Categories 是否在跳过列表中 | ||
| static bool shouldSkipCgroupsByCategories(const QString &desktopId, QAbstractItemModel *activeAppModel, const QHash<int, QByteArray> &roleNames) |
There was a problem hiding this comment.
倒不如直接把 “尝试通过AM(Application Manager)匹配应用程序” 那块提出来。只提目前这一部分有点怪怪的。
|
TAG Bot New tag: 2.0.25 |
|
recheck |
|
TAG Bot New tag: 2.0.26 |
3b87e6d to
62a5b07
Compare
62a5b07 to
6fe519b
Compare
1. 新增 DConfig 配置项 cgroupsBasedGroupingSkipCategories 2. 默认跳过 TerminalEmulator 类别的应用,避免终端中启动的程序被错误识别 3. 在 dde-apps 的 AppItem 中添加 categories 属性 4. 在 AppItemModel 中添加 CategoriesRole 5. 在 AMAppItem 构造函数中保存从 AM 获取的 categories 6. 提取 tryMatchByApplicationManager() 函数,优化代码结构 7. 将 categories 检查逻辑合并到 tryMatchByApplicationManager() 中 8. 使用 Q_ASSERT 确保 model 参数有效性 9. 使用 TaskManager::CategoriesRole 枚举常量替代运行时查找 10. 优化迭代逻辑,迭代 skipCategories 而非 categories,提升性能 11. 在窗口匹配流程中集成类别检查逻辑 Log: 新增基于应用类别的 cgroups 分组跳过列表,解决终端中启动的图形程序被错误归类的问题 Influence: 1. 测试在 Alacritty 等终端模拟器中启动图形应用 2. 验证新启动的应用在任务栏显示独立图标 3. 验证 categories 从 dde-apps 缓存读取,无额外 DBus 调用 4. 检查 DConfig 配置项可正常读取和修改 5. 确认不影响非终端类应用的正常分组行为 6. 验证深度终端等原有白名单应用仍正常工作 7. 确认代码结构优化不影响功能正确性 PMS: TASK-384865
6fe519b to
44d8682
Compare
deepin pr auto review代码审查报告总体评估这段代码为任务管理器添加了基于应用程序类别进行分组过滤的功能,允许用户配置某些类别的应用程序不参与基于cgroups的任务分组。代码结构清晰,功能实现完整,但存在一些可以改进的地方。 详细分析1. 语法和逻辑优点
改进建议
2. 代码质量优点
改进建议
3. 代码性能优点
改进建议
4. 代码安全优点
改进建议
总结这段代码整体质量良好,实现了所需功能。主要改进方向包括:
这些改进不会改变代码的核心功能,但能提高代码的可维护性、性能和安全性。 |
|
[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. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
概述
新增基于应用类别(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: