chore: make hover preview proxy model simpler #1346
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
简化 HoverPreviewProxyModel, 使 model 本身不与设置项耦合, 过滤规则 在 taskmanager.cpp 控制 Log:
Reviewer's GuideThis PR refactors the hover preview proxy by removing its dependency on TaskManagerSettings and an index-based API, introducing a simple string filter with a FilterMode enum, and shifting filter decision logic into TaskManager::requestPreview. Sequence diagram for new filter logic in TaskManager::requestPreviewsequenceDiagram
participant TaskManager
participant HoverPreviewProxyModel
TaskManager->>HoverPreviewProxyModel: setFilter(winId, FilterByWinId) (if windowSplit)
TaskManager->>HoverPreviewProxyModel: setFilter(appId, FilterByAppId) (else)
HoverPreviewProxyModel->>HoverPreviewProxyModel: invalidateFilter()
TaskManager->>HoverPreviewProxyModel: rowCount()
Class diagram for refactored HoverPreviewProxyModelclassDiagram
class HoverPreviewProxyModel {
+setFilter(QString filter, FilterMode mode)
+clearFilter()
+filterAcceptsRow(int sourceRow, const QModelIndex &sourceParent) const
-FilterMode m_filterMode
-QString m_filter
}
HoverPreviewProxyModel --|> QSortFilterProxyModel
class FilterMode {
<<enumeration>>
FilterByAppId
FilterByWinId
}
HoverPreviewProxyModel o-- FilterMode
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
deepin pr auto review我来对这段代码变更进行审查：
建议改进：
case FilterByWinId: {
bool ok;
uint32_t targetWinId = m_filter.toUInt(&ok);
if (!ok || targetWinId == 0) {
return false;
}
return winId == targetWinId;
}
void HoverPreviewProxyModel::setFilter(QString filter, enum FilterMode mode)
{
if (mode == FilterByWinId) {
bool ok;
filter.toUInt(&ok);
if (!ok) {
qWarning() << "Invalid window ID filter:" << filter;
return;
}
}
m_filter = filter;
m_filterMode = mode;
invalidateFilter();
}
void HoverPreviewProxyModel::setFilter(QString filter, enum FilterMode mode)
{
if (filter.isEmpty()) {
clearFilter();
return;
}
// ... 其余代码
}总体来说，这是一个很好的重构，提高了代码的可维护性和性能，但需要加强输入验证来提高代码的健壮性。 |
![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:
- Initialize m_filter and m_filterMode in the constructor initializer list to avoid any uninitialized state when filterAcceptsRow is first called.
- Update the doxygen comment above setFilter to reflect the new QString and FilterMode parameters instead of the old QModelIndex API.
- Consider renaming FilterByAppId to something like FilterByDesktopId for clarity, since it’s actually matching on DesktopIdRole rather than an application identifier.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Initialize m_filter and m_filterMode in the constructor initializer list to avoid any uninitialized state when filterAcceptsRow is first called.
- Update the doxygen comment above setFilter to reflect the new QString and FilterMode parameters instead of the old QModelIndex API.
- Consider renaming FilterByAppId to something like FilterByDesktopId for clarity, since it’s actually matching on DesktopIdRole rather than an application identifier.
## Individual Comments
### Comment 1
<location> `panels/dock/taskmanager/hoverpreviewproxymodel.cpp:60-61` </location>
<code_context>
+ return currentDesktopId == m_filter;
+ }
+ case FilterByWinId: {
+ uint32_t targetWinId = m_filter.toUInt();
bool result = (winId == targetWinId && targetWinId != 0);
- // 在 WindowSplit 模式下，精确匹配单个窗口
- return result;
</code_context>
<issue_to_address>
**issue (bug_risk):** Potential silent conversion issue when using QString::toUInt().
If m_filter is not numeric, toUInt() returns 0, which may cause incorrect filtering. Please validate m_filter or handle conversion errors.
</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
+60
to
61
| uint32_t targetWinId = m_filter.toUInt(); | ||
| bool result = (winId == targetWinId && targetWinId != 0); |
There was a problem hiding this comment.
issue (bug_risk): Potential silent conversion issue when using QString::toUInt().
If m_filter is not numeric, toUInt() returns 0, which may cause incorrect filtering. Please validate m_filter or handle conversion errors.
wjyrich
approved these changes
Nov 13, 2025
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: BLumia, 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
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.
简化 HoverPreviewProxyModel, 使 model 本身不与设置项耦合, 过滤规则 在 taskmanager.cpp 控制
Summary by Sourcery
Simplify hover preview filtering by moving rule control to TaskManager and streamlining the proxy model's API and logic
Enhancements: