From: Frank Reininghaus Date: Sat, 7 Sep 2013 16:06:45 +0000 (+0200) Subject: Make expandedParentsCount() work without accessing the data hash X-Git-Url: https://cloud.milkyroute.net/gitweb/dolphin.git/commitdiff_plain/7e30467679a403f953e2547894febb51e8a3d78b Make expandedParentsCount() work without accessing the data hash The idea is that we no longer assume that the "expandedParentsCount" for each item will be stored in the QHash. It is only accessed for items which are expanded, and which are not top-level items (i.e., which have an expandedParentsCount > 1). Some unit tests are added to improve the coverage of the affected code. REVIEW: 112562 --- diff --git a/src/kitemviews/kfileitemmodel.cpp b/src/kitemviews/kfileitemmodel.cpp index 37a30519a..51359bbce 100644 --- a/src/kitemviews/kfileitemmodel.cpp +++ b/src/kitemviews/kfileitemmodel.cpp @@ -494,10 +494,7 @@ bool KFileItemModel::isExpandable(int index) const int KFileItemModel::expandedParentsCount(int index) const { if (index >= 0 && index < count()) { - const int parentsCount = m_itemData.at(index)->values.value("expandedParentsCount").toInt(); - if (parentsCount > 0) { - return parentsCount; - } + return expandedParentsCount(m_itemData.at(index)); } return 0; } @@ -1234,6 +1231,23 @@ QList KFileItemModel::createItemDataList(const KUrl& return itemDataList; } +int KFileItemModel::expandedParentsCount(const ItemData* data) +{ + // The hash 'values' is only guaranteed to contain the key "expandedParentsCount" + // if the corresponding item is expanded, and it is not a top-level item. + const ItemData* parent = data->parent; + if (parent) { + if (parent->parent) { + Q_ASSERT(parent->values.contains("expandedParentsCount")); + return parent->values.value("expandedParentsCount").toInt() + 1; + } else { + return 1; + } + } else { + return 0; + } +} + void KFileItemModel::removeExpandedItems() { KFileItemList expandedItems; @@ -1241,15 +1255,12 @@ void KFileItemModel::removeExpandedItems() const int maxIndex = m_itemData.count() - 1; for (int i = 0; i <= maxIndex; ++i) { const ItemData* itemData = m_itemData.at(i); - if (itemData->values.value("expandedParentsCount").toInt() > 0) { + if (itemData->parent) { expandedItems.append(itemData->item); } } - // The m_expandedParentsCountRoot may not get reset before all items with - // a bigger count have been removed. removeItems(expandedItems, DeleteItemData); - m_expandedDirs.clear(); } @@ -1394,7 +1405,7 @@ QHash KFileItemModel::retrieveData(const KFileItem& item, if (m_requestRole[ExpandedParentsCountRole]) { if (parent) { - const int level = parent->values["expandedParentsCount"].toInt() + 1; + const int level = expandedParentsCount(parent) + 1; data.insert(sharedValue("expandedParentsCount"), level); } } @@ -1418,8 +1429,8 @@ bool KFileItemModel::lessThan(const ItemData* a, const ItemData* b) const int result = 0; if (a->parent != b->parent) { - const int expansionLevelA = a->values.value("expandedParentsCount").toInt(); - const int expansionLevelB = b->values.value("expandedParentsCount").toInt(); + const int expansionLevelA = expandedParentsCount(a); + const int expansionLevelB = expandedParentsCount(b); // If b has a higher expansion level than a, check if a is a parent // of b, and make sure that both expansion levels are equal otherwise. @@ -1439,7 +1450,7 @@ bool KFileItemModel::lessThan(const ItemData* a, const ItemData* b) const a = a->parent; } - Q_ASSERT(a->values.value("expandedParentsCount").toInt() == b->values.value("expandedParentsCount").toInt()); + Q_ASSERT(expandedParentsCount(a) == expandedParentsCount(b)); // Compare the last parents of a and b which are different. while (a->parent != b->parent) { @@ -1938,9 +1949,9 @@ KFileItemList KFileItemModel::childItems(const KFileItem& item) const int index = m_items.value(item.url(), -1); if (index >= 0) { - const int parentLevel = m_itemData.at(index)->values.value("expandedParentsCount").toInt(); + const int parentLevel = expandedParentsCount(index); ++index; - while (index < m_itemData.count() && m_itemData.at(index)->values.value("expandedParentsCount").toInt() > parentLevel) { + while (index < m_itemData.count() && expandedParentsCount(index) > parentLevel) { items.append(m_itemData.at(index)->item); ++index; } @@ -2075,7 +2086,7 @@ bool KFileItemModel::isConsistent() const const ItemData* data = m_itemData.at(i); const ItemData* parent = data->parent; if (parent) { - if (data->values.value("expandedParentsCount").toInt() != parent->values.value("expandedParentsCount").toInt() + 1) { + if (expandedParentsCount(data) != expandedParentsCount(parent) + 1) { qWarning() << "expandedParentsCount is inconsistent for parent" << parent->item << "and child" << data->item; return false; } diff --git a/src/kitemviews/kfileitemmodel.h b/src/kitemviews/kfileitemmodel.h index 5917e6818..f364e7ce0 100644 --- a/src/kitemviews/kfileitemmodel.h +++ b/src/kitemviews/kfileitemmodel.h @@ -319,6 +319,8 @@ private: */ QList createItemDataList(const KUrl& parentUrl, const KFileItemList& items) const; + static int expandedParentsCount(const ItemData* data); + void removeExpandedItems(); /** diff --git a/src/tests/kfileitemmodeltest.cpp b/src/tests/kfileitemmodeltest.cpp index e55e3eb33..1d9646b3e 100644 --- a/src/tests/kfileitemmodeltest.cpp +++ b/src/tests/kfileitemmodeltest.cpp @@ -486,7 +486,8 @@ void KFileItemModelTest::testExpandItems() // KFileItemModel::expansionLevelsCompare(const KFileItem& a, const KFileItem& b) // yields the correct result for "a/a/1" and "a/a-1/", whis is non-trivial because they share the // first three characters. - QSet modelRoles = m_model->roles(); + QSet originalModelRoles = m_model->roles(); + QSet modelRoles = originalModelRoles; modelRoles << "isExpanded" << "isExpandable" << "expandedParentsCount"; m_model->setRoles(modelRoles); @@ -593,6 +594,18 @@ void KFileItemModelTest::testExpandItems() QVERIFY(QTest::kWaitForSignal(m_model, SIGNAL(directoryLoadingCompleted()), DefaultTimeout)); QCOMPARE(m_model->count(), 5); // 5 items: "a/", "a/a/", "a/a/1", "a/a-1/", "a/a-1/1" QCOMPARE(m_model->expandedDirectories(), allFolders); + + // Remove all expanded items by changing the roles + spyRemoved.clear(); + m_model->setRoles(originalModelRoles); + QVERIFY(!m_model->isExpanded(0)); + QCOMPARE(m_model->count(), 1); + QVERIFY(!m_model->expandedDirectories().contains(KUrl(m_testDir->name() + 'a'))); + + QCOMPARE(spyRemoved.count(), 1); + itemRangeList = spyRemoved.takeFirst().at(0).value(); + QCOMPARE(itemRangeList, KItemRangeList() << KItemRange(1, 4)); // 4 items removed + QVERIFY(m_model->isConsistent()); } void KFileItemModelTest::testExpandParentItems() @@ -645,6 +658,28 @@ void KFileItemModelTest::testExpandParentItems() QVERIFY(m_model->isExpanded(3)); QVERIFY(!m_model->isExpanded(4)); QVERIFY(m_model->isConsistent()); + + // Expand "a 1/b1/". + m_model->setExpanded(1, true); + QVERIFY(QTest::kWaitForSignal(m_model, SIGNAL(directoryLoadingCompleted()), DefaultTimeout)); + QCOMPARE(m_model->count(), 6); + QVERIFY(m_model->isExpanded(0)); + QVERIFY(m_model->isExpanded(1)); + QVERIFY(!m_model->isExpanded(2)); + QVERIFY(m_model->isExpanded(3)); + QVERIFY(m_model->isExpanded(4)); + QVERIFY(!m_model->isExpanded(5)); + QVERIFY(m_model->isConsistent()); + + // Collapse "a 1/b1/" again, and verify that the previous state is restored. + m_model->setExpanded(1, false); + QCOMPARE(m_model->count(), 5); + QVERIFY(m_model->isExpanded(0)); + QVERIFY(!m_model->isExpanded(1)); + QVERIFY(m_model->isExpanded(2)); + QVERIFY(m_model->isExpanded(3)); + QVERIFY(!m_model->isExpanded(4)); + QVERIFY(m_model->isConsistent()); } /**