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
