diff options
author | Samuel Gaist <samuel.gaist@edeltech.ch> | 2017-10-05 16:20:36 +0200 |
---|---|---|
committer | Samuel Gaist <samuel.gaist@edeltech.ch> | 2017-10-17 12:28:23 +0000 |
commit | 1382374deaa4a854aeb542e6c8f7e1841f2abb10 (patch) | |
tree | 872b27c6d22475e70cec47d75f52ce44fe19a60c | |
parent | d0a0a3c0418a0fdd2ed84b2a5f7e6565537715c6 (diff) |
Correct QStandardItemModel::setItemData to follow QAbstractItemModel
QStandardItemModel::setItemData replaces the content of an item data
with the new values rather than updating/inserting which is
the behavior for QAbstractItemModel. This patch aims to unify the
behavior.
[ChangeLog][QtWidgets][QStandardItemModel] Fixed setItemData()
incorrectly deleting unmodified data. That behavior is not
following QAbstractItemModel's documented behavior which is
no modification of data not provided in parameter.
Task-number: QTBUG-45114
Task-number: QTBUG-10872
Change-Id: I2be40cee372b68d9f71c976548ecda6dc3011241
Reviewed-by: David Faure <david.faure@kdab.com>
-rw-r--r-- | src/gui/itemmodels/qstandarditemmodel.cpp | 93 | ||||
-rw-r--r-- | src/gui/itemmodels/qstandarditemmodel_p.h | 11 | ||||
-rw-r--r-- | tests/auto/gui/itemmodels/qstandarditemmodel/tst_qstandarditemmodel.cpp | 62 |
3 files changed, 156 insertions, 10 deletions
diff --git a/src/gui/itemmodels/qstandarditemmodel.cpp b/src/gui/itemmodels/qstandarditemmodel.cpp index 07e372b1ae..519995e82a 100644 --- a/src/gui/itemmodels/qstandarditemmodel.cpp +++ b/src/gui/itemmodels/qstandarditemmodel.cpp @@ -185,6 +185,72 @@ void QStandardItemPrivate::childDeleted(QStandardItem *child) emit model->dataChanged(modelIndex, modelIndex); } +namespace { + + struct ByNormalizedRole + { + static int normalizedRole(int role) + { + return role == Qt::EditRole ? Qt::DisplayRole : role; + } + + bool operator()(const QStandardItemData& standardItemData, const std::pair<const int &, const QVariant&>& roleMapIt) const + { + return standardItemData.role < normalizedRole(roleMapIt.first); + } + bool operator()(const std::pair<const int&, const QVariant &>& roleMapIt, const QStandardItemData& standardItemData) const + { + return normalizedRole(roleMapIt.first) < standardItemData.role; + } + + }; + + /* + Based on std::transform with a twist. The inputs are iterators of <int, QVariant> pair. + The variant is checked for validity and if not valid, that element is not taken into account + which means that the resulting output might be shorter than the input. + */ + template<class Input, class OutputIt> + OutputIt roleMapStandardItemDataTransform(Input first1, Input last1, OutputIt d_first) + { + while (first1 != last1) { + if ((*first1).second.isValid()) + *d_first++ = QStandardItemData(*first1); + ++first1; + } + return d_first; + } + + + /* + Based on std::set_union with a twist. The idea is to create a union of both inputs + with an additional constraint: if an input contains an invalid variant, it means + that this one should not be taken into account for generating the output. + */ + template<class Input1, class Input2, + class OutputIt, class Compare> + OutputIt roleMapStandardItemDataUnion(Input1 first1, Input1 last1, + Input2 first2, Input2 last2, + OutputIt d_first, Compare comp) + { + for (; first1 != last1; ++d_first) { + if (first2 == last2) { + return roleMapStandardItemDataTransform(first1, last1, d_first); + } + if (comp(*first2, *first1)) { + *d_first = *first2++; + } else { + if ((*first1).second.isValid()) + *d_first = QStandardItemData(*first1); + if (!comp(*first1, *first2)) + ++first2; + ++first1; + } + } + return std::copy(first2, last2, d_first); + } +} + /*! \internal */ @@ -192,18 +258,25 @@ void QStandardItemPrivate::setItemData(const QMap<int, QVariant> &roles) { Q_Q(QStandardItem); - //let's build the vector of new values + auto byRole = [](const QStandardItemData& item1, const QStandardItemData& item2) { + return item1.role < item2.role; + }; + + std::sort(values.begin(), values.end(), byRole); + + /* + Create a vector of QStandardItemData that will contain the original values + if the matching role is not contained in roles, the new value if it is and + if the new value is an invalid QVariant, it will be removed. + */ QVector<QStandardItemData> newValues; - for (auto it = roles.begin(), end = roles.end(); it != end; ++it) { - const QVariant &value = it.value(); - if (value.isValid()) { - int role = it.key(); - role = (role == Qt::EditRole) ? Qt::DisplayRole : role; - newValues.append(QStandardItemData(role, value)); - } - } + newValues.reserve(values.size()); + roleMapStandardItemDataUnion(roles.keyValueBegin(), + roles.keyValueEnd(), + values.cbegin(), values.cend(), + std::back_inserter(newValues), ByNormalizedRole()); - if (values!=newValues) { + if (newValues != values) { values.swap(newValues); if (model) model->d_func()->itemChanged(q); diff --git a/src/gui/itemmodels/qstandarditemmodel_p.h b/src/gui/itemmodels/qstandarditemmodel_p.h index 516cce8613..caee3ea34c 100644 --- a/src/gui/itemmodels/qstandarditemmodel_p.h +++ b/src/gui/itemmodels/qstandarditemmodel_p.h @@ -61,6 +61,7 @@ #include <QtCore/qstack.h> #include <QtCore/qvariant.h> #include <QtCore/qvector.h> +#include <QtCore/qdebug.h> QT_BEGIN_NAMESPACE @@ -69,6 +70,7 @@ class QStandardItemData public: inline QStandardItemData() : role(-1) {} inline QStandardItemData(int r, const QVariant &v) : role(r), value(v) {} + inline QStandardItemData(const std::pair<const int&, const QVariant&> &p) : role(p.first), value(p.second) {} int role; QVariant value; inline bool operator==(const QStandardItemData &other) const { return role == other.role && value == other.value; } @@ -91,6 +93,15 @@ inline QDataStream &operator<<(QDataStream &out, const QStandardItemData &data) return out; } +inline QDebug &operator<<(QDebug &debug, const QStandardItemData &data) +{ + QDebugStateSaver saver(debug); + debug.nospace() << data.role + << " " + << data.value; + return debug.space(); +} + #endif // QT_NO_DATASTREAM class QStandardItemPrivate diff --git a/tests/auto/gui/itemmodels/qstandarditemmodel/tst_qstandarditemmodel.cpp b/tests/auto/gui/itemmodels/qstandarditemmodel/tst_qstandarditemmodel.cpp index 2f5537adfe..a07f45ef1d 100644 --- a/tests/auto/gui/itemmodels/qstandarditemmodel/tst_qstandarditemmodel.cpp +++ b/tests/auto/gui/itemmodels/qstandarditemmodel/tst_qstandarditemmodel.cpp @@ -130,6 +130,8 @@ private slots: void getMimeDataWithInvalidModelIndex(); void supportedDragDropActions(); + void taskQTBUG_45114_setItemData(); + private: QAbstractItemModel *m_model; QPersistentModelIndex persistent; @@ -1701,5 +1703,65 @@ void tst_QStandardItemModel::supportedDragDropActions() QCOMPARE(model.supportedDropActions(), Qt::CopyAction | Qt::MoveAction); } +void tst_QStandardItemModel::taskQTBUG_45114_setItemData() +{ + QStandardItemModel model; + QSignalSpy spy(&model, &QStandardItemModel::itemChanged); + + QStandardItem *item = new QStandardItem("item"); + item->setData(1, Qt::UserRole + 1); + item->setData(2, Qt::UserRole + 2); + model.appendRow(item); + + QModelIndex index = item->index(); + QCOMPARE(model.itemData(index).size(), 3); + + QCOMPARE(spy.count(), 0); + + QMap<int, QVariant> roles; + + roles.insert(Qt::UserRole + 1, 1); + roles.insert(Qt::UserRole + 2, 2); + model.setItemData(index, roles); + + QCOMPARE(spy.count(), 0); + + roles.insert(Qt::UserRole + 1, 1); + roles.insert(Qt::UserRole + 2, 2); + roles.insert(Qt::UserRole + 3, QVariant()); + model.setItemData(index, roles); + + QCOMPARE(spy.count(), 0); + + roles.clear(); + roles.insert(Qt::UserRole + 1, 10); + roles.insert(Qt::UserRole + 3, 12); + model.setItemData(index, roles); + + QCOMPARE(spy.count(), 1); + QMap<int, QVariant> itemRoles = model.itemData(index); + + QCOMPARE(itemRoles.size(), 4); + QCOMPARE(itemRoles[Qt::UserRole + 1].toInt(), 10); + QCOMPARE(itemRoles[Qt::UserRole + 2].toInt(), 2); + QCOMPARE(itemRoles[Qt::UserRole + 3].toInt(), 12); + + roles.clear(); + roles.insert(Qt::UserRole + 3, 1); + model.setItemData(index, roles); + + QCOMPARE(spy.count(), 2); + + roles.clear(); + roles.insert(Qt::UserRole + 3, QVariant()); + model.setItemData(index, roles); + + QCOMPARE(spy.count(), 3); + + itemRoles = model.itemData(index); + QCOMPARE(itemRoles.size(), 3); + QVERIFY(!itemRoles.keys().contains(Qt::UserRole + 3)); +} + QTEST_MAIN(tst_QStandardItemModel) #include "tst_qstandarditemmodel.moc" |