D25326: Move Plasma's SortFilterProxyModel into KItemModel's QML plugin

[Arjen Hiemstra]

ahiemstra added inline comments.

INLINE COMMENTS

> kmaterka wrote in ksortfilterproxymodel_qml.cpp:111
> Can you add a test for sortColumn? It might be useful for newcomers (like me) to understand how it works. I had real trouble understanding when it is needed and when not (ConfigEntries.qml <https://phabricator.kde.org/source/plasma-workspace/browse/master/applets/systemtray/package/contents/ui/ConfigEntries.qml;v5.17.90$83>)

I don't think a unit test is the right place for example code. It's probably better to either improve the documentation of the property or add an example somewhere where it makes sense.

> kmaterka wrote in ksortfilterproxymodel.cpp:165
> When using PlasmaCore.SortFilterModel sortColumn is sometimes set to -1 and sorting is not working when `sortColumn` is not set. If I remember correctly, -1 is the default in QSortFilterProxyModel. Is `std::max(sortColumn(), 0)` added to fix that? What will happen in this situation:
> 
>   KSortFilterProxyModel {
>     sourceModel: testModel
>     sortColumn: -1
>     sortRole: "user"
>   }
> 
> Maybe is should be documented in `sortRole` and `sortOrder` properties that these two set sortColumn to 0 (or -1 in case of empty role)?

In that example, you'd be sorting on the "user" role of column 0, which seems like a reasonable default to me. I would actually suggest to make sortColumn always at least 0, I don't think there really is much of a use case for -1 as default.

> kmaterka wrote in ksortfilterproxymodel.h:67
> can we have something similar for sorting? `sortColumnCallback` and use it in overridden `lessThan`?

Let's keep this a bit constrained, if we add a stub implementation of `lessThan()`, we can later on add a property to expose a JS callback for it somehow.

REPOSITORY
  R275 KItemModels

REVISION DETAIL
  https://phabricator.kde.org/D25326

To: davidedmundson
Cc: kmaterka, iasensio, ahmadsamir, broulik, ahiemstra, mart, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20200129/c9824f75/attachment.html>