Repository navigation
fix: replace private isVariantLessThan with QCollator, fix deprecated invalidateFilter - #840
Conversation
Reviewer's GuideThe PR removes the Qt 6.12-incompatible private comparison API and replaces it with numeric handling plus QCollator fallback, while version-gating filter refresh calls to avoid deprecation failures on newer Qt and preserve Qt 6.8 compatibility. Sequence diagram for version-gated filter refreshsequenceDiagram
participant Model as MultipageSortFilterProxyModel
participant Qt612 as Qt_6_12_or_newer
participant Qt68 as Qt_6_8
Model->>Model: onFolderIdChanged or onPageIdChanged
alt Qt 6.12 or newer
Model->>Qt612: beginFilterChange()
Model->>Qt612: endFilterChange()
else Qt 6.8
Model->>Qt68: invalidateFilter()
end
Flow diagram for locale-aware model sortingflowchart TD
A[SortProxyModel.lessThan] --> B{Value type}
B -->|String| C[QString::compare]
B -->|Numeric| D[Direct numeric comparison]
B -->|Other| E[QCollator::compare]
E --> F[Locale-aware ordering]
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 found 3 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src/models/sortproxymodel.cpp" line_range="265" />
<code_context>
+ || lhs.typeId() == QMetaType::Double || lhs.typeId() == QMetaType::Float)
+ {
+ // Numeric types compare directly
+ return lhs.toLongLong() < rhs.toLongLong();
+ }
else
</code_context>
<issue_to_address>
**Numeric sort keys lose their order**
When a sort role contains distinct fractional values or unsigned values greater than `LLONG_MAX`, `lessThan()` converts numeric variants with `toLongLong()`, truncating fractional values and failing to preserve the full range of unsigned values. Distinct numeric keys can compare equal or in the wrong order, so the proxy displays rows in the wrong order.
Compare numeric variants without converting all values to `long long`, preserving floating-point and unsigned ordering.
</issue_to_address>
### Comment 2
<location path="src/models/sortproxymodel.cpp" line_range="23" />
<code_context>
#include <QVariant>
#include <QLoggingCategory>
-#include <private/qabstractitemmodel_p.h>
+#include <QCollator>
</code_context>
<issue_to_address>
**Private Qt build dependency remains**
When building in an environment that provides Qt public development files but not Qt private development files, the model target still requires and links `Qt::CorePrivate`, so builds without the Qt private development package continue to fail despite removing the private-header include.
Remove the now-unneeded CorePrivate package and link requirements from the model and test targets, after checking for other consumers in those targets.
</issue_to_address>
### Comment 3
<location path="src/models/sortproxymodel.cpp" line_range="272-275" />
<code_context>
+ // QAbstractItemModelPrivate::isVariantLessThan() was removed from the
+ // private headers in Qt 6.12; use QCollator for locale-aware string
+ // comparison as the fallback (matching the original behavior).
+ QCollator collator;
+ collator.setCaseSensitivity(m_caseSensitivity);
+ return collator.compare(lhs.toString(), rhs.toString()) < 0;
}
}
</code_context>
<issue_to_address>
**String values bypass locale collation**
When the sort role returns `QString` values, `lessThan()` returns through its earlier `QString::compare` branch, so string values never reach the `QCollator` fallback and retain code-unit ordering instead of the intended locale-aware ordering.
Route `QString` values through `QCollator` instead of the earlier string-comparison branch.
</issue_to_address>| #include <QVariant> | ||
| #include <QLoggingCategory> | ||
|
|
||
| #include <private/qabstractitemmodel_p.h> |
There was a problem hiding this comment.
🟡 Medium · Private Qt build dependency remains
When building in an environment that provides Qt public development files but not Qt private development files, the model target still requires and links Qt::CorePrivate, so builds without the Qt private development package continue to fail despite removing the private-header include.
Remove the now-unneeded CorePrivate package and link requirements from the model and test targets, after checking for other consumers in those targets.
Prompt for AI agents
In `src/models/sortproxymodel.cpp` at line 23:
**Private Qt build dependency remains**
When building in an environment that provides Qt public development files but not Qt private development files, the model target still requires and links `Qt::CorePrivate`, so builds without the Qt private development package continue to fail despite removing the private-header include.
Remove the now-unneeded CorePrivate package and link requirements from the model and test targets, after checking for other consumers in those targets.74ea90b to
f32cded
Compare
…ix deprecated invalidateFilter 1. QAbstractItemModelPrivate::isVariantLessThan() was removed from the private headers in Qt 6.12, causing a compile error in sortproxymodel.cpp. Replace with: numeric roles compare via toDouble(), string roles compare via QCollator (locale-aware, improves Chinese/pinyin sorting). Drop the private header. 2. QSortFilterProxyModel::invalidateFilter() is deprecated since Qt 6.13; guard both call sites in multipagesortfilterproxymodel.cpp with a QT_VERSION check using beginFilterChange()/endFilterChange() from Qt 6.12, keeping invalidateFilter() for the Qt 6.8 baseline. Log: dde-launchpad builds on both Qt 6.8 and Qt 6.12 without private API dependencies or deprecation errors Influence: 1. Build on Qt 6.8 and Qt 6.12, confirm no compile errors 2. Verify launcher app list sorting by name (Chinese + Latin) and numeric roles (frequency, timestamps) works correctly fix: 以数值/字符串比较替代私有 isVariantLessThan;修复 invalidateFilter 弃用 1. Qt 6.12 将 QAbstractItemModelPrivate::isVariantLessThan() 从 私有头文件移除,导致 sortproxymodel.cpp 编译错误。替代方案: 数值角色通过 toDouble() 比较,字符串角色通过 QCollator (本地化感知,改善中文拼音排序)比较。移除私有头依赖。 2. QSortFilterProxyModel::invalidateFilter() 自 Qt 6.13 起废弃; 对 multipagesortfilterproxymodel.cpp 中两处调用加 QT_VERSION 门控,Qt 6.12 起使用 beginFilterChange()/endFilterChange(), Qt 6.8 基线保留旧路径。 Log: dde-launchpad 在 Qt 6.8 和 Qt 6.12 下均可构建 Influence: 1. 在 Qt 6.8 和 Qt 6.12 下构建,确认无编译错误 2. 验证启动器应用列表按名称(中文 + 拉丁)及数值角色排序正常
f32cded to
c309284
Compare
deepin pr auto review🤖 AI 代码审查报告📊 总体评价
🔍 详细分析1. 语法逻辑 ❌评价: 存在逻辑缺陷 ❌ 不通过 潜在问题: 建议: ['对于非字符串类型,建议使用 QVariant::compare()(Qt 6)进行通用比较,或至少检查 canConvert() 后再调用 toDouble(),对不可转换的类型回退到 toString() 比较'] 2. 代码质量 ✅评价: 代码结构清晰,注释完整 ✅ 通过 潜在问题: 建议: ['将 beginFilterChange()/endFilterChange() 与 invalidateFilter() 的版本兼容逻辑提取为单独的辅助方法,如 void invalidateFilterCompat()', '统一预处理指令的缩进风格,建议 #if/#else/#endif 均顶格或均缩进'] 3. 代码性能 ❌评价: 存在性能问题 ❌ 不通过 潜在问题: 建议: ['将 QCollator 声明为类的成员变量(如 m_collator),在构造函数或 setSortCaseSensitivity 中初始化/更新,lessThan 中直接使用 m_collator.compare()'] 4. 代码安全 🔒评价: 存在0个安全漏洞 ✅ 通过
安全漏洞详情: 建议: [] 💡 改进建议代码示例// sortproxymodel.h - 添加成员变量
private:
QCollator m_collator;
// sortproxymodel.cpp - 构造函数中初始化
SortProxyModel::SortProxyModel(QObject *parent)
: QSortFilterProxyModel(parent)
{
m_collator.setCaseSensitivity(m_caseSensitivity);
// ... 其他初始化
}
// sortproxymodel.cpp - lessThan 改进
bool SortProxyModel::lessThan(const QModelIndex &source_left, const QModelIndex &source_right) const
{
const QVariant lhs = source_left.data(m_sortRole);
const QVariant rhs = source_right.data(m_sortRole);
if (lhs.typeId() != QMetaType::QString || rhs.typeId() != QMetaType::QString) {
#if QT_VERSION >= QT_VERSION_CHECK(6, 0, 0)
// Qt 6: 使用 QVariant::compare 进行通用类型比较
const auto cmp = QVariant::compare(lhs, rhs);
return cmp == QPartialOrdering::Less;
#else
// Qt 5: 优先数值比较,回退字符串比较
bool ok1 = false, ok2 = false;
const double d1 = lhs.toDouble(&ok1);
const double d2 = rhs.toDouble(&ok2);
if (ok1 && ok2)
return d1 < d2;
return lhs.toString() < rhs.toString();
#endif
}
// 使用缓存的 m_collator 进行字符串比较
return m_collator.compare(lhs.toString(), rhs.toString()) < 0;
}本报告由 AI 代码审查工具自动生成 |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 18202781743, pengfeixx 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 |
|
/forcemerge |
|
This pr force merged! (status: blocked) |
Summary
Two Qt 6.12 compatibility fixes in
src/models/:1. Private API removal (compile error)
Qt 6.12 removed
QAbstractItemModelPrivate::isVariantLessThan()from the private headers. Replace with:#include <private/qabstractitemmodel_p.h>dependency entirely2. Deprecated API (warning → error with -Werror)
Guard both call sites with
QT_VERSIONcheck: usebeginFilterChange()/endFilterChange()from Qt 6.12, keepinvalidateFilter()for the 6.8 baseline.Changes
src/models/sortproxymodel.cpp(+14/-2): QCollator + numeric comparison replaces private API; remove private header includesrc/models/multipagesortfilterproxymodel.cpp(+10): version-gated begin/endFilterChange at 2 call sitesTesting
Summary by Sourcery
Update model sorting and filtering for Qt 6.12 compatibility while preserving support for the Qt 6.8 baseline.
Bug Fixes:
Enhancements: