feat: add category-based skip list for cgroups grouping #1384
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: |
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 |
1. 新增 DConfig 配置项 cgroupsBasedGroupingSkipCategories 2. 默认跳过 TerminalEmulator 类别的应用，避免终端中启动的程序被错误识别 3. 实现 shouldSkipCgroupsByCategories() 函数，通过 DBus 查询应用类别 4. 在窗口匹配流程中集成类别检查逻辑 Log: 新增基于应用类别的 cgroups 分组跳过列表，解决终端中启动的图形程序被错误归类的问题 Influence: 1. 测试在 Alacritty 等终端模拟器中启动图形应用 2. 验证新启动的应用在任务栏显示独立图标 3. 检查 DConfig 配置项可正常读取和修改 4. 确认不影响非终端类应用的正常分组行为 5. 验证深度终端等原有白名单应用仍正常工作 PMS: TASK-384865
…r factory 1. 使用 DesktopFileParserFactory 替代直接 DBus 调用 2. 在 DesktopfileAbstractParser 中添加 categories() 虚方法 3. 在 DesktopFileAMParser 中实现 categories() 方法 4. 利用工厂的对象缓存机制，提升性能 5. 移除直接的 Application DBus 接口创建代码 Log: 重构基于类别的 cgroups 跳过实现，使用 Parser 工厂模式替代直接 DBus 调用，提升代码复用性和性能 Influence: 1. 功能保持不变，仍然正确识别终端模拟器类应用 2. 利用 Parser 对象缓存，减少重复创建开销 3. 代码架构更统一，与现有 Parser 体系集成 4. 验证在 Alacritty 等终端中启动的应用仍显示独立图标 5. 确认性能优化不影响功能正确性 PMS: TASK-384865
1. 在 dde-apps 的 AppItem 中添加 categories 属性 2. 在 AppItemModel 中添加 CategoriesRole 3. 在 AMAppItem 构造函数中保存从 AM 获取的 categories 4. 修改 shouldSkipCgroupsByCategories() 函数，从 dde-apps 读取 categories 5. 移除对即将废弃的 DESKTOPFILEFACTORY 的依赖 6. 回退 desktopfileabstractparser 和 desktopfileamparser 中的 categories 方法 Log: 实现基于应用类别的 cgroups 分组跳过功能，默认跳过 TerminalEmulator 类别应用 Influence: 1. 测试在终端模拟器中启动图形应用，验证应用显示为独立图标 2. 验证 categories 从 dde-apps 缓存读取，无额外 DBus 调用 3. 确认不影响非终端类应用的正常分组行为 4. 验证 DConfig 配置项可正常读取和修改 PMS: TASK-384865
Ivy233
force-pushed
the
feature/cgroups-skip-categories
branch
from
a0e2e66 to
3b87e6d
Compare
deepin pr auto review我来对这个diff进行详细的代码审查：
优点：
需要改进的地方： a) 性能优化： // 在 shouldSkipCgroupsByCategories 函数中
static bool shouldSkipCgroupsByCategories(const QString &desktopId, QAbstractItemModel *activeAppModel, const QHash<int, QByteArray> &roleNames)
{
auto skipCategories = Settings->cgroupsBasedGroupingSkipCategories();
if (skipCategories.isEmpty() || desktopId.isEmpty()) {
return false;
}
// 建议：将categories转换成QSet，提高查找效率
QSet<QString> skipCategoriesSet(skipCategories.begin(), skipCategories.end());
// ... 其余代码
// 使用QSet::contains替代QList::contains
for (const QString &category : categories) {
if (skipCategoriesSet.contains(category)) {
qCDebug(taskManagerLog) << "Skipping cgroups grouping for" << desktopId
<< "due to category:" << category;
return true;
}
}
}b) 安全性：
c) 错误处理： // 建议在读取categories时添加错误处理
if (existingItem.isValid()) {
auto categoriesData = activeAppModel->data(existingItem, roleNames.key("categories"));
if (categoriesData.isValid()) {
categories = categoriesData.toStringList();
}
}d) 代码复用： // 建议将获取categories的逻辑抽取为独立函数
static QStringList getAppCategories(const QString &desktopId, QAbstractItemModel *activeAppModel, const QHash<int, QByteArray> &roleNames)
{
if (!activeAppModel || desktopId.isEmpty()) {
return {};
}
auto existingItem = activeAppModel->match(activeAppModel->index(0, 0), roleNames.key("desktopId"), desktopId, 1, Qt::MatchFixedString | Qt::MatchWrap).value(0);
if (existingItem.isValid()) {
auto categoriesData = activeAppModel->data(existingItem, roleNames.key("categories"));
if (categoriesData.isValid()) {
return categoriesData.toStringList();
}
}
return {};
}
总的来说，这个实现基本满足了功能需求，但在性能优化、错误处理和代码复用方面还有改进空间。建议在后续版本中逐步完善这些方面。 |
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 |
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: