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

[David Edmundson]

davidedmundson marked 3 inline comments as done.
davidedmundson added inline comments.

INLINE COMMENTS

> broulik wrote in sortfiltermodel.cpp:84
> Can we also just have it send `source_row` and `source_parent` since we have proper `QModelIndex` handling now?

We could.

Though means everyone doing a lookup immediately, handling columns and roles properly.

More powerful, but slightly more complex for the user

There's also the option of having two proxies with different behaviours?

@mart thoughts?

> broulik wrote in sortfiltermodel.h:43
> Can this be a `QRegularExpression`?
> I would assume a JS `RegExp` object (or regexp literal `/syntax/`) was supported (didn't test)

It's not listed in https://doc.qt.io/qt-5/qtqml-cppintegration-data.html

We need to test

> davidedmundson wrote in sortfiltermodel.h:63
> Or potentially we could just kill our handling of it?

Did some investigation we have our handling just to emit a signal when the property changes!

Another good thing to try to upstream.

Keeping the same name allows us to subtly drop the API in the future and keep clients working.

> broulik wrote in sortfiltermodel.h:78
> Is this needed?

I think so. We've implemented it in a lot of plasma models, presumably for some reason.

Would be a good thing to try and upstream though.

> broulik wrote in sortfiltermodel.h:138
> Does this need a `d` pointer or is it just for use in QML? Perhaps rename the header to `_p.h` then?

I don't see why we would need a d pointer. We may as well self an allocation.

> Perhaps rename the header to _p.h then?

We haven't done that universally in all frameworks, but sure I could.

REPOSITORY
  R275 KItemModels

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

To: davidedmundson
Cc: 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/20191118/b44b32d1/attachment.html>