davidedmundson added a comment.
Looks good. 1 tiny change needed I think.
Also some code comments on the QVector would be useful for future people.
INLINE COMMENTS
> taskgroupingproxymodel.cpp:41
>
> - QVector<QVector<int>> rowMap;
> + QVector<QVector<int> *> rowMap;
>
Cleanup on destruction.
(which sounds like the name of a new Megadeth single)
> taskgroupingproxymodel.cpp:465
>
> - rowMap[row].resize(1);
> -
> - // index() encodes parent row numbers in indices returned for group
> - // members. endRemoveRows() is not aware of this scheme, and so won't
> - // invalidate persistent indices for the members of the group we're
> - // dissolving. We're now going to do it ourselves.
> - foreach (const QModelIndex &idx, q->persistentIndexList()) {
> - if (idx.internalId() == ((uint)row + 1)) {
> - q->changePersistentIndex(idx, QModelIndex());
> - }
> - }
> + rowMap[row]->resize(1);
>
(I know this existing code)
why 1?
shouldn't this should be currentSize + extraChildren.count()
> taskgroupingproxymodel.cpp:521
> + return index(parentRow, 0);
> + }
> + }
I think we should assert after this if.
REPOSITORY
R120 Plasma Workspace
REVISION DETAIL
https://phabricator.kde.org/D7139
To: hein, #plasma, davidedmundson
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg,
abetts, sebas, apol, mart, lukas
