broulik added inline comments. INLINE COMMENTS
> davidedmundson wrote in sortfiltermodel.h:43 > It's not listed in https://doc.qt.io/qt-5/qtqml-cppintegration-data.html > > We need to test Bummer. I tried, a JS `RegExp` (both `new RegExp()` and `/literal syntax/`) get turned into a `QRegExp`. It does *not* work with `QRegularExpression` :( There's a `RegularExpressionValidator` which uses `QRegularExpression` but it's in Qt 5.14. Maybe they fixed that recently - doesn't really help us here, though. > davidedmundson wrote in sortfiltermodel.h:63 > 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. > Another good thing to try to upstream. +1 given they're all `QObject`s already anyway > davidedmundson wrote in sortfiltermodel.h:78 > 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. > presumably for some reason. Either because someone was too lazy to use the respective property on the `Repeater` or `ListView` or before `rowCount()` was `Q_INVOKABLE`. Not sure it's upstreamable since it assumes it's a flast list. Maybe something for QAbstract*List*Model rather than QAbstract*Item*Model > davidedmundson wrote in sortfiltermodel.h:138 > 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. Was just wondering but if it's only to be used from QML and not exported, doesn't need one I suppose. 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
