This revision was automatically updated to reflect the committed changes.
Closed by commit R275:4fd7a211bb8d: Move Plasma's SortFilterProxyModel
into KItemModel's QML plugin (authored by ahiemstra, committed by
davidedmundson).
CHANGED PRIOR TO COMMIT
https://phabricator.kde.org/D25326?vs=7460
broulik accepted this revision.
This revision is now accepted and ready to land.
REPOSITORY
R275 KItemModels
BRANCH
arcpatch-D25326
REVISION DETAIL
https://phabricator.kde.org/D25326
To: davidedmundson, broulik
Cc: kmaterka, iasensio, ahmadsamir, broulik, ahiemstra, mart,
kde-frameworks-
ahiemstra updated this revision to Diff 74601.
ahiemstra added a comment.
- Use QQmlParserStatus to slightly delay syncing of role names
REPOSITORY
R275 KItemModels
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D25326?vs=74592&id=74601
BRANCH
arcpatch-D25326
REVISION DETAIL
davidedmundson updated this revision to Diff 74592.
davidedmundson marked 14 inline comments as done.
davidedmundson added a comment.
a load of review comments
REPOSITORY
R275 KItemModels
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D25326?vs=71421&id=74592
BRANCH
master
REVI
ahiemstra added inline comments.
INLINE COMMENTS
> ksortfilterproxymodel.cpp:74
> +QJSValueList args = {QJSValue(source_row),
> engine->toScriptValue(source_parent)};
> +return const_cast *>(this)->m_filterCallback.call(args).toBool();
> +}
Suggestion: Instead of directly re
ahiemstra added inline comments.
INLINE COMMENTS
> ksortfilterproxymodel.cpp:85
> +QJSValueList args = {QJSValue(source_column),
> engine->toScriptValue(source_parent)};
> +return const_cast *>(this)->m_filterCallback.call(args).toBool();
> +}
This is using the wrong callbac
ahiemstra added a comment.
Something I noticed just now while trying to use this: the `filterString`
property has disappeared. PlasmaCore.SortFilterProxy does have this. I think we
should restore it, otherwise you need to manually call
QSFPM::setFilterFixedString or hope that you can set it
kmaterka added inline comments.
INLINE COMMENTS
> ahiemstra wrote in ksortfilterproxymodel_qml.cpp:111
> 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.
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.q
kmaterka added a comment.
Few comments from someone who was recently using `PlasmaCore.SortFilterModel`
and had trouble understanding sorting :)
INLINE COMMENTS
> ksortfilterproxymodel_qml.cpp:111
> +
> +void tst_KSortFilterProxyModelQml::testSortRole()
> +{
Can you add a test for sortColum
ahmadsamir added inline comments.
INLINE COMMENTS
> davidedmundson wrote in ksortfilterproxymodel.cpp:101
> I'm not sure your comments are still valid, we switched to using the
> superclass QSortFilterProxyModel's methods, and don't shadow that part here.
Right, I was looking at an old revision
davidedmundson added inline comments.
INLINE COMMENTS
> ahmadsamir wrote in ksortfilterproxymodel.cpp:101
> In setFilterRegExp(),
> QSortFilterProxyModel::setFilterRegularExpression(QRegularExpression &) is
> used, so IIUC here it should be :
> QSortFilterProxyModel::filterRegularExpression().p
ahmadsamir added inline comments.
INLINE COMMENTS
> ksortfilterproxymodel.cpp:101
> +{
> +return QSortFilterProxyModel::filterRegExp().pattern();
> +}
In setFilterRegExp(),
QSortFilterProxyModel::setFilterRegularExpression(QRegularExpression &) is
used, so IIUC here it should be :
QSortFil
davidedmundson updated this revision to Diff 71421.
davidedmundson edited the summary of this revision.
davidedmundson added a comment.
Huge cleanup
Drop all the filter properties
Implement other changes
Extend unit tests
REPOSITORY
R275 KItemModels
CHANGES SINCE LAST UPDATE
http
davidedmundson added inline comments.
INLINE COMMENTS
> broulik wrote in ksortfilterproxymodel.h:44
> Did you try that `QRegularExpression` vs `QRegExp` thing?
We did some research
- Old Qt which we support doesn't convert JS to QRegularExpresssion
- We don't want to write new code with QRegExp
broulik added inline comments.
INLINE COMMENTS
> ksortfilterproxymodel.h:2
> +/*
> + * Copyright 2010 by Marco MArtin
> +
*Martin
> ksortfilterproxymodel.h:44
> + */
> +Q_PROPERTY(QString filterRegExp READ filterRegExp WRITE setFilterRegExp
> NOTIFY filterRegExpChanged)
> +
Did you
davidedmundson updated this revision to Diff 71329.
davidedmundson added a comment.
Implement new arguments for filterAcceptsRow passing source_row,
source_parent.
It makes for slightly heavier messier JS, but it also allows for super
flexibility
beyond just having the value retried a
davidedmundson updated this revision to Diff 71327.
davidedmundson added a comment.
update
REPOSITORY
R275 KItemModels
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D25326?vs=71326&id=71327
BRANCH
master
REVISION DETAIL
https://phabricator.kde.org/D25326
AFFECTED FILES
au
davidedmundson updated this revision to Diff 71326.
davidedmundson added a comment.
All changes except the parameters of filterRowCallback
REPOSITORY
R275 KItemModels
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D25326?vs=69918&id=71326
BRANCH
master
REVISION DETAIL
https:/
davidedmundson added inline comments.
INLINE COMMENTS
> broulik wrote in sortfiltermodel.h:78
> > 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 upstr
davidedmundson marked an inline comment as done.
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
davidedmundson updated this revision to Diff 69918.
davidedmundson marked 3 inline comments as done.
davidedmundson added a comment.
asdf
REPOSITORY
R275 KItemModels
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D25326?vs=69917&id=69918
BRANCH
master
REVISION DETAIL
https://
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`.
davidedmundson updated this revision to Diff 69917.
davidedmundson edited the summary of this revision.
davidedmundson added a comment.
Only some changes done, not all
Updating now to share added unit test
REPOSITORY
R275 KItemModels
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org
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 d
broulik added inline comments.
INLINE COMMENTS
> sortfiltermodel.cpp:84
> +QQmlEngine *engine = QQmlEngine::contextForObject(this)->engine();
> +args <<
> engine->toScriptValue(idx.data(m_roleIds.value(m_filterRole)));
> +
Can we also just have it send `source_row` and `source_p
davidedmundson added inline comments.
INLINE COMMENTS
> ahiemstra wrote in sortfiltermodel.h:63
> This (and sortRole below) hides the properties from QSortFilterProxy with the
> same name. Maybe it is a better idea to use filterRoleName/sortRoleName?
Or potentially we could just kill our handli
ahiemstra added inline comments.
INLINE COMMENTS
> sortfiltermodel.h:63
> + */
> +Q_PROPERTY(QString filterRole READ filterRole WRITE setFilterRole)
> +
This (and sortRole below) hides the properties from QSortFilterProxy with the
same name. Maybe it is a better idea to use filterRoleNa
davidedmundson edited the summary of this revision.
REPOSITORY
R275 KItemModels
REVISION DETAIL
https://phabricator.kde.org/D25326
To: davidedmundson
Cc: mart, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
mart added a comment.
+1: i found myself many times wanting to use this but not being able because
of plasma dep.
the code is used a lot in plasmoids and seems quite robust
REPOSITORY
R275 KItemModels
REVISION DETAIL
https://phabricator.kde.org/D25326
To: davidedmundson
Cc: mart, kde-
davidedmundson created this revision.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
davidedmundson requested review of this revision.
REVISION SUMMARY
It exposes QSFPM in a usable way from QML, but also exposes a way to
perform JS callbacks as an advanced
31 matches
Mail list logo