feat: implement close all windows functionality #1281
Merged
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
Reviewer's GuideIntroduce a new requestCloseAll method in DockGlobalElementModel to batch-close grouped application windows by desktop ID using X11Utils, update the CLOSEALL action to use this method, and add necessary interface declarations and includes. Sequence diagram for the new close all windows functionalitysequenceDiagram
participant User as actor User
participant DockGlobalElementModel
participant DockCombineModel
participant X11Utils
User->>DockGlobalElementModel: Trigger CLOSEALL action
DockGlobalElementModel->>DockCombineModel: Get windows with matching desktop ID
DockCombineModel-->>DockGlobalElementModel: Return list of window IDs
loop For each window ID
DockGlobalElementModel->>X11Utils: closeWindow(winId)
end
Updated class diagram for DockGlobalElementModel and related classesclassDiagram
class DockGlobalElementModel {
+void requestClose(const QModelIndex &index, bool force = false) const
+void requestCloseAll(const QModelIndex &index) const
+void requestUpdateWindowGeometry(const QModelIndex &index, const QRect &geometry, QObject *delegate = nullptr) const
}
class DockCombineModel {
+int rowCount()
+QModelIndex index(int row, int column)
}
class X11Utils {
+static X11Utils* instance()
+void closeWindow(uint32_t winId)
}
DockGlobalElementModel --> DockCombineModel : uses
DockGlobalElementModel --> X11Utils : uses
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 there - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `panels/dock/taskmanager/dockglobalelementmodel.cpp:419-420` </location>
<code_context>
+ QModelIndex idx = combineModel->index(i, 0);
+ QString desktopId = idx.data(TaskManager::DesktopIdRole).toString();
+ if (desktopId == targetDesktopId) {
+ uint32_t winId = idx.data(TaskManager::WinIdRole).toUInt();
+ if (winId != 0) {
+ windowIds.append(winId);
+ }
</code_context>
<issue_to_address>
**nitpick:** Zero winId is skipped without explanation.
Clarify in the code why winId == 0 is skipped, or add handling for cases where 0 may be valid.
</issue_to_address>
### Comment 2
<location> `panels/dock/taskmanager/dockglobalelementmodel.cpp:426-427` </location>
<code_context>
+ }
+
+ for (uint32_t winId : windowIds) {
+ X11Utils::instance()->closeWindow(winId);
+ }
+ }
</code_context>
<issue_to_address>
**suggestion (bug_risk):** No error handling for failed window close operations.
Add error handling or logging to capture failures from X11Utils::closeWindow, so issues are not missed.
```suggestion
for (uint32_t winId : windowIds) {
if (!X11Utils::instance()->closeWindow(winId)) {
qWarning() << "Failed to close window with ID:" << winId;
}
```
</issue_to_address>
### Comment 3
<location> `panels/dock/taskmanager/dockglobalelementmodel.cpp:404` </location>
<code_context>
qWarning() << "unable to close app not running";
}
+
+void DockGlobalElementModel::requestCloseAll(const QModelIndex &index) const
+{
+ auto data = m_data.value(index.row());
</code_context>
<issue_to_address>
**issue (complexity):** Consider moving the window-closing logic into a DockCombineModel helper to simplify requestCloseAll to a single method call.
You can collapse almost all of the nested loops and X11 calls into a single helper on `DockCombineModel`, then keep `requestCloseAll` down to one cast-and-dispatch. For example:
In DockCombineModel (new method):
```cpp
// dockgroupmodel.h / dockgroupmodel.cpp
void DockCombineModel::closeWindowsOnDesktop(const QString &desktopId) const
{
QList<uint32_t> ids;
for (int row = 0; row < rowCount(); ++row) {
const auto idx = index(row, 0);
if (idx.data(TaskManager::DesktopIdRole).toString() != desktopId)
continue;
uint32_t win = idx.data(TaskManager::WinIdRole).toUInt();
if (win)
ids << win;
}
for (auto win : ids)
X11Utils::instance()->closeWindow(win);
}
```
Then in DockGlobalElementModel::requestCloseAll, replace all of the manual filtering with:
```cpp
void DockGlobalElementModel::requestCloseAll(const QModelIndex &index) const
{
auto [id, sourceModel, sourceRow] = m_data.value(index.row());
if (auto combine = qobject_cast<DockCombineModel*>(sourceModel)) {
const QString desktopId = sourceModel->index(sourceRow, 0)
.data(TaskManager::DesktopIdRole)
.toString();
combine->closeWindowsOnDesktop(desktopId);
}
}
```
This keeps “close-all” logic in your model, removes deep nesting in DockGlobalElementModel, and preserves exactly the same behavior.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
419
to
420
| uint32_t winId = idx.data(TaskManager::WinIdRole).toUInt(); | ||
| if (winId != 0) { |
There was a problem hiding this comment.
nitpick: Zero winId is skipped without explanation.
Clarify in the code why winId == 0 is skipped, or add handling for cases where 0 may be valid.
| qWarning() << "unable to close app not running"; | ||
| } | ||
|
|
||
| void DockGlobalElementModel::requestCloseAll(const QModelIndex &index) const |
There was a problem hiding this comment.
issue (complexity): Consider moving the window-closing logic into a DockCombineModel helper to simplify requestCloseAll to a single method call.
You can collapse almost all of the nested loops and X11 calls into a single helper on DockCombineModel, then keep requestCloseAll down to one cast-and-dispatch. For example:
In DockCombineModel (new method):
// dockgroupmodel.h / dockgroupmodel.cpp
void DockCombineModel::closeWindowsOnDesktop(const QString &desktopId) const
{
QList<uint32_t> ids;
for (int row = 0; row < rowCount(); ++row) {
const auto idx = index(row, 0);
if (idx.data(TaskManager::DesktopIdRole).toString() != desktopId)
continue;
uint32_t win = idx.data(TaskManager::WinIdRole).toUInt();
if (win)
ids << win;
}
for (auto win : ids)
X11Utils::instance()->closeWindow(win);
}Then in DockGlobalElementModel::requestCloseAll, replace all of the manual filtering with:
void DockGlobalElementModel::requestCloseAll(const QModelIndex &index) const
{
auto [id, sourceModel, sourceRow] = m_data.value(index.row());
if (auto combine = qobject_cast<DockCombineModel*>(sourceModel)) {
const QString desktopId = sourceModel->index(sourceRow, 0)
.data(TaskManager::DesktopIdRole)
.toString();
combine->closeWindowsOnDesktop(desktopId);
}
}This keeps “close-all” logic in your model, removes deep nesting in DockGlobalElementModel, and preserves exactly the same behavior.
BLumia
reviewed
Sep 28, 2025
| requestClose(index, true); | ||
| } else if (action == DOCK_ACTION_CLOSEALL) { | ||
| requestClose(index); | ||
| requestCloseAll(index); |
Member
There was a problem hiding this comment.
是不是直接把 DockGlobalElementModel::requestNewInstance 的实现往上挪一层比较合适？在 DockGroupModel 提供 requestNewInstance 的实现，然后这里就可以去掉了。
cc @tsic404
wjyrich
force-pushed
the
fix-bug-334659
branch
from
bdb4e84 to
2da7b5e
Compare
18202781743
previously approved these changes
Sep 29, 2025
1. Add requestCloseAll method to handle close all action for grouped applications 2. When close all is triggered, find all windows with the same desktop ID and close them 3. Use X11Utils to send close commands to individual windows 4. Add necessary includes for DockGroupModel and X11Utils 5. Update action handler to call requestCloseAll instead of requestClose for close all action feat: 实现关闭所有窗口功能 1. 添加 requestCloseAll 方法来处理分组应用的关闭所有操作 2. 当触发关闭所有时，查找具有相同桌面 ID 的所有窗口并关闭它们 3. 使用 X11Utils 向各个窗口发送关闭命令 4. 添加 DockGroupModel 和 X11Utils 的必要包含 5. 更新操作处理器，为关闭所有操作调用 requestCloseAll 而不是 requestClose Pms: BUG-334659
wjyrich
force-pushed
the
fix-bug-334659
branch
from
cfa1563 to
84d6ba9
Compare
deepin pr auto review我来分析这段代码的变更和改进： 1. 代码结构变更
2. 改进点分析优点：
需要改进的地方：
3. 具体改进建议// 1. 添加错误处理
void DockGroupModel::requestNewInstance(const QModelIndex &index, const QString &action) const {
if (action == DOCK_ACTION_DOCK) {
auto desktopId = index.data(TaskManager::DesktopIdRole).toString();
TaskManagerSettings::instance()->toggleDockedElement(QStringLiteral("desktop/%1").arg(desktopId));
} else if (action == DOCK_ACTION_FORCEQUIT) {
requestClose(index, true);
} else if (action == DOCK_ACTION_CLOSEALL) {
requestClose(index);
} else {
auto desktopId = index.data(TaskManager::DesktopIdRole).toString();
QProcess process;
process.setProcessChannelMode(QProcess::MergedChannels);
// 验证 action 参数
static const QStringList allowedActions = {"action1", "action2", "action3"}; // 替换为实际允许的操作
if (!allowedActions.contains(action)) {
qWarning() << "Invalid action requested:" << action;
return;
}
process.start("dde-am", {"--by-user", desktopId, action});
if (!process.waitForFinished(5000)) { // 设置超时
qWarning() << "Process execution timed out or failed:" << process.errorString();
return;
}
if (process.exitCode() != 0) {
qWarning() << "Process exited with error:" << process.readAllStandardError();
}
}
}
// 2. 使用异步处理
// 可以考虑使用 QFuture 或信号槽机制来处理进程执行
// 3. 定义常量
class DockGroupModel : public RoleGroupModel, public AbstractTaskManagerInterface {
private:
static constexpr const char* DESKTOP_ID_PREFIX = "desktop/%1";
// ...
};
// 4. 提取公共方法
void DockGroupModel::handleProcessAction(const QString &desktopId, const QString &action) const {
QProcess process;
process.setProcessChannelMode(QProcess::MergedChannels);
process.start("dde-am", {"--by-user", desktopId, action});
// ... 错误处理
}4. 总结这次代码重构在职责分离和类型安全方面做出了改进，但在错误处理、性能优化和安全性方面还有提升空间。建议按照上述建议进行改进，以提高代码的健壮性、安全性和可维护性。 |
18202781743
approved these changes
Sep 29, 2025
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 18202781743, wjyrich 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 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 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.
feat: 实现关闭所有窗口功能
Pms: BUG-334659
Summary by Sourcery
Implement close-all windows action for grouped applications
New Features:
Enhancements: