fix: WinSCP dock icon stayed when window closed #1340
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
修复关闭 WinSCP 窗口时,dock 上仍会有一个残留的 WinSCP 图标的问题. 注:仍然注意到这个问题可能导致其他窗口 hover preview 数量变多,出现重 复窗口.此问题待继续排查. Log:
Reviewer's GuideThis PR refactors DockGlobalElementModel to use queued connections for all model-change signals and streamlines the rowsRemoved/rowsInserted logic to correctly prune stale entries (e.g. WinSCP icons) and adjust indices, while enhancing dataChanged handling to include ItemIdRole when desktop or identity roles update. Sequence diagram for dock icon removal on window closesequenceDiagram
participant "QAbstractItemModel (m_activeAppModel)"
participant "DockGlobalElementModel"
participant "Dock UI"
"QAbstractItemModel (m_activeAppModel)"->>"DockGlobalElementModel": rowsRemoved (QueuedConnection)
activate "DockGlobalElementModel"
"DockGlobalElementModel"->>"DockGlobalElementModel": Prune stale entries from m_data
"DockGlobalElementModel"->>"Dock UI": beginRemoveRows / endRemoveRows
deactivate "DockGlobalElementModel"
Class diagram for DockGlobalElementModel signal handling changesclassDiagram
class DockGlobalElementModel {
- m_appsModel: QAbstractItemModel*
- m_activeAppModel: QAbstractItemModel*
- m_data: QVector<Tuple>
+ DockGlobalElementModel(QAbstractItemModel*, DockActiveAppModel*)
+ loadDockedElements()
<<signal handling>>
+ connect rowsRemoved (QueuedConnection)
+ connect rowsInserted (QueuedConnection)
+ connect dataChanged (QueuedConnection)
}
class QAbstractItemModel {
<<Qt Model>>
+ rowsRemoved
+ rowsInserted
+ dataChanged
}
class DockActiveAppModel {
<<inherits QAbstractItemModel>>
}
DockGlobalElementModel o-- QAbstractItemModel : uses
DockGlobalElementModel o-- DockActiveAppModel : uses
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
deepin pr auto review我来分析一下这个diff的改进建议：
总体来说，这次改动主要是代码格式化和线程安全性的改进，是一个好的重构。建议在后续版本中考虑上述其他改进点。 |
![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 - here's some feedback:
- Refactor the duplicated lambdas handling rowsInserted/rowsRemoved into helper functions to reduce repetition and improve readability.
- Replace the std::tuple entries in m_data with a dedicated struct having named fields to make the code more self-documenting.
- Add a brief note or assertion around using Qt::QueuedConnection to clarify the intended update ordering and guard against subtle race conditions.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Refactor the duplicated lambdas handling rowsInserted/rowsRemoved into helper functions to reduce repetition and improve readability.
- Replace the std::tuple entries in m_data with a dedicated struct having named fields to make the code more self-documenting.
- Add a brief note or assertion around using Qt::QueuedConnection to clarify the intended update ordering and guard against subtle race conditions.
## Individual Comments
### Comment 1
<location> `panels/dock/taskmanager/dockglobalelementmodel.cpp:32` </location>
<code_context>
, m_activeAppModel(activeAppModel)
{
connect(TaskManagerSettings::instance(), &TaskManagerSettings::dockedElementsChanged, this, &DockGlobalElementModel::loadDockedElements);
- connect(m_appsModel, &QAbstractItemModel::rowsRemoved, this, [this](const QModelIndex &parent, int first, int last) {
- Q_UNUSED(parent)
- for (int i = first; i <= last; ++i) {
</code_context>
<issue_to_address>
**issue (complexity):** Consider extracting repeated lambda logic into private helper functions to simplify and reduce duplication in signal handlers.
You have four very similar lambdas here doing almost the same thing (find-and-remove, find-and-update, shift indices, emit dataChanged, etc.). You can collapse a lot of that into small private helpers and reduce each lambda to a single call. For example, extract the “shift everything after row X by ±N” logic into:
```cpp
// in DockGlobalElementModel.h
private:
void shiftRowIndices(QAbstractItemModel *src, int first, int last, int delta);
// in DockGlobalElementModel.cpp
void DockGlobalElementModel::shiftRowIndices(QAbstractItemModel *src,
int first, int last,
int delta)
{
for (auto &entry : m_data) {
if (std::get<1>(entry) == src && std::get<2>(entry) >= first) {
std::get<2>(entry) += delta;
}
}
}
```
Then your `rowsRemoved` for `m_appsModel` becomes:
```cpp
connect(m_appsModel, &QAbstractItemModel::rowsRemoved, this,
[this](const QModelIndex&, int first, int last) {
// remove any exact-match rows
for (int i = first; i <= last; ++i) {
auto it = std::find_if(m_data.begin(), m_data.end(),
[this, i](auto &d){ return std::get<1>(d) == m_appsModel
&& std::get<2>(d) == i; });
if (it != m_data.end()) {
int pos = it - m_data.begin();
beginRemoveRows(QModelIndex(), pos, pos);
m_data.remove(pos);
endRemoveRows();
}
}
// shift everything after “first” down by (last-first+1)
shiftRowIndices(m_appsModel, first, last, -((last-first)+1));
},
Qt::QueuedConnection);
```
You can apply the same pattern to
- insert (use `shiftRowIndices(..., +((last-first)+1))`),
- active-model removal (extract its special remove/replace logic into e.g. `handleActiveRowsRemoved(...)`),
- dataChanged (just find & emit).
This cuts out ~20–30 lines of duplicated lambdas and makes each `connect()` a one-liner to a helper.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
tsic404
approved these changes
Nov 11, 2025
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: BLumia, tsic404 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
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.
修复关闭 WinSCP 窗口时,dock 上仍会有一个残留的 WinSCP 图标的问题. （相关帖子）
注:仍然注意到这个问题可能导致其他窗口 hover preview 数量变多,出现重复窗口.此问题待继续排查.
Summary by Sourcery
Improve DockGlobalElementModel signal-slot connections by switching to queued connections and adding missing removal handling on the active apps model to prevent stale dock icons when windows close.
Bug Fixes:
Enhancements: