From ed83f37f06bd5c4b38c100503c3ad4dec2a87912 Mon Sep 17 00:00:00 2001 From: Eduardo Cruz Date: Mon, 4 Oct 2021 07:13:54 +0000 Subject: [PATCH] Rewrite filter algorithm to properly support filtering with expanded folders under Detail View mode. BUG: 411878 CCBUG: 442275 FIXED-IN: 21.12 --- src/kitemviews/kfileitemmodel.cpp | 82 +++++++++++++++++++++++-------- src/tests/kfileitemmodeltest.cpp | 38 +++++++++----- 2 files changed, 88 insertions(+), 32 deletions(-) diff --git a/src/kitemviews/kfileitemmodel.cpp b/src/kitemviews/kfileitemmodel.cpp index bf2597903..d12355100 100644 --- a/src/kitemviews/kfileitemmodel.cpp +++ b/src/kitemviews/kfileitemmodel.cpp @@ -695,45 +695,87 @@ QStringList KFileItemModel::mimeTypeFilters() const return m_filter.mimeTypes(); } - void KFileItemModel::applyFilters() { - // Check which shown items from m_itemData must get - // hidden and hence moved to m_filteredItems. - QVector newFilteredIndexes; + // ===STEP 1=== + // Check which previously shown items from m_itemData must now get + // hidden and hence moved from m_itemData into m_filteredItems. - const int itemCount = m_itemData.count(); - for (int index = 0; index < itemCount; ++index) { - ItemData* itemData = m_itemData.at(index); + QList newFilteredIndexes; // This structure is good for prepending. We will want an ascending sorted Container at the end, this will do fine. - // Only filter non-expanded items as child items may never - // exist without a parent item - if (!itemData->values.value("isExpanded").toBool()) { - const KFileItem item = itemData->item; - if (!m_filter.matches(item)) { - newFilteredIndexes.append(index); - m_filteredItems.insert(item, itemData); - } + // This pointer will refer to the next confirmed shown item from the point of + // view of the current "itemData" in the upcoming "for" loop. + ItemData *itemShownBelow = nullptr; + + // We will iterate backwards because it's convenient to know beforehand if the item just below is its child or not. + for (int index = m_itemData.count() - 1; index >= 0; --index) { + ItemData *itemData = m_itemData.at(index); + + if (m_filter.matches(itemData->item) + || (itemShownBelow && itemShownBelow->parent == itemData && itemData->values.value("isExpanded").toBool())) { + // We could've entered here for two reasons: + // 1. This item passes the filter itself + // 2. This is an expanded folder that doesn't pass the filter but sees a filter-passing child just below + + // So this item must remain shown. + // Lets register this item as the next shown item from the point of view of the next iteration of this for loop + itemShownBelow = itemData; + } else { + // We hide this item for now, however, for expanded folders this is not final: + // if after the next "for" loop we discover that its children must now be shown with the newly applied fliter, we shall re-insert it + newFilteredIndexes.prepend(index); + m_filteredItems.insert(itemData->item, itemData); + // indexShownBelow doesn't get updated since this item will be hidden } } - const KItemRangeList removedRanges = KItemRangeList::fromSortedContainer(newFilteredIndexes); - removeItems(removedRanges, KeepItemData); + // This will remove the newly filtered items from m_itemData + removeItems(KItemRangeList::fromSortedContainer(newFilteredIndexes), KeepItemData); + // ===STEP 2=== // Check which hidden items from m_filteredItems should - // get visible again and hence removed from m_filteredItems. - QList newVisibleItems; + // become visible again and hence moved from m_filteredItems back into m_itemData. - QHash::iterator it = m_filteredItems.begin(); + QList newVisibleItems; + + QHash ancestorsOfNewVisibleItems; // We will make sure these also become visible in step 3. + + QHash::iterator it = m_filteredItems.begin(); while (it != m_filteredItems.end()) { if (m_filter.matches(it.key())) { newVisibleItems.append(it.value()); + + // If this is a child of an expanded folder, we must make sure that its whole parental chain will also be shown. + // We will go up through its parental chain until we either: + // 1 - reach the "root item" of the current view, i.e the currently opened folder on Dolphin. Their children have their ItemData::parent set to nullptr. + // or + // 2 - we reach an unfiltered parent or a previously discovered ancestor. + for (ItemData *parent = it.value()->parent; parent && !ancestorsOfNewVisibleItems.contains(parent->item) && m_filteredItems.contains(parent->item); + parent = parent->parent) { + // We wish we could remove this parent from m_filteredItems right now, but we are iterating over it + // and it would mess up the iteration. We will mark it to be removed in step 3. + ancestorsOfNewVisibleItems.insert(parent->item, parent); + } + it = m_filteredItems.erase(it); } else { + // Item remains filtered for now + // However, for expanded folders this is not final, we may discover later that it has unfiltered descendants. ++it; } } + // ===STEP 3=== + // Handles the ancestorsOfNewVisibleItems. + // Now that we are done iterating through m_filteredItems we can safely move the ancestorsOfNewVisibleItems from m_filteredItems to newVisibleItems. + for (it = ancestorsOfNewVisibleItems.begin(); it != ancestorsOfNewVisibleItems.end(); it++) { + if (m_filteredItems.remove(it.key())) { + // m_filteredItems still contained this ancestor until now so we can be sure that we aren't adding a duplicate ancestor to newVisibleItems. + newVisibleItems.append(it.value()); + } + } + + // This will insert the newly discovered unfiltered items into m_itemData insertItems(newVisibleItems); } diff --git a/src/tests/kfileitemmodeltest.cpp b/src/tests/kfileitemmodeltest.cpp index 6a7c4cca6..7a22a1a7f 100644 --- a/src/tests/kfileitemmodeltest.cpp +++ b/src/tests/kfileitemmodeltest.cpp @@ -1226,11 +1226,16 @@ void KFileItemModelTest::collapseParentOfHiddenItems() QVERIFY(itemsInsertedSpy.wait()); QCOMPARE(m_model->count(), 7); // 7 items: "a/", "a/b/", "a/b/c", "a/b/c/d/", "a/b/c/1", "a/b/1", "a/1" - // Set a name filter that matches nothing -> only the expanded folders remain. + // Set a name filter that matches nothing -> nothing should remain. m_model->setNameFilter("xyz"); QCOMPARE(itemsRemovedSpy.count(), 1); - QCOMPARE(m_model->count(), 3); - QCOMPARE(itemsInModel(), QStringList() << "a" << "b" << "c"); + QCOMPARE(m_model->count(), 0); //Everything is hidden + QCOMPARE(itemsInModel(), QStringList()); + + //Filter by the file names. Folder "d" will be hidden since it was collapsed + m_model->setNameFilter("1"); + QCOMPARE(itemsRemovedSpy.count(), 1); // nothing was removed, itemsRemovedSpy count will remain the same: + QCOMPARE(m_model->count(), 6); // 6 items: "a/", "a/b/", "a/b/c", "a/b/c/1", "a/b/1", "a/1" // Collapse the folder "a/". m_model->setExpanded(0, false); @@ -1238,9 +1243,11 @@ void KFileItemModelTest::collapseParentOfHiddenItems() QCOMPARE(m_model->count(), 1); QCOMPARE(itemsInModel(), QStringList() << "a"); - // Remove the filter -> no files should appear (and we should not get a crash). + // Remove the filter -> "a" should still appear (and we should not get a crash). m_model->setNameFilter(QString()); + QCOMPARE(itemsRemovedSpy.count(), 2); // nothing was removed, itemsRemovedSpy count will remain the same: QCOMPARE(m_model->count(), 1); + QCOMPARE(itemsInModel(), QStringList() << "a"); } /** @@ -1276,9 +1283,15 @@ void KFileItemModelTest::removeParentOfHiddenItems() QVERIFY(itemsInsertedSpy.wait()); QCOMPARE(m_model->count(), 7); // 7 items: "a/", "a/b/", "a/b/c", "a/b/c/d/", "a/b/c/1", "a/b/1", "a/1" - // Set a name filter that matches nothing -> only the expanded folders remain. + // Set a name filter that matches nothing -> nothing should remain. m_model->setNameFilter("xyz"); QCOMPARE(itemsRemovedSpy.count(), 1); + QCOMPARE(m_model->count(), 0); + QCOMPARE(itemsInModel(), QStringList()); + + // Filter by "c". Folder "b" will also be shown because it is its parent. + m_model->setNameFilter("c"); + QCOMPARE(itemsRemovedSpy.count(), 1); // nothing was removed, itemsRemovedSpy count will remain the same: QCOMPARE(m_model->count(), 3); QCOMPARE(itemsInModel(), QStringList() << "a" << "b" << "c"); @@ -1349,21 +1362,22 @@ void KFileItemModelTest::testGeneralParentChildRelationships() m_model->slotCompleted(); QCOMPARE(itemsInModel(), QStringList() << "parent1" << "realChild1" << "grandChild1" << "realGrandChild1" << "child1" << "parent2" << "realChild2" << "realGrandChild2" << "child2"); - m_model->slotItemsAdded(realChild1, KFileItemList() << KFileItem(QUrl("grandChild1"), QString(), KFileItem::Unknown)); - m_model->slotCompleted(); - QCOMPARE(itemsInModel(), QStringList() << "parent1" << "realChild1" << "grandChild1" << "realGrandChild1" << "child1" << "parent2" << "realChild2" << "realGrandChild2" << "child2"); - m_model->slotItemsAdded(realChild2, KFileItemList() << KFileItem(QUrl("grandChild2"), QString(), KFileItem::Unknown)); m_model->slotCompleted(); QCOMPARE(itemsInModel(), QStringList() << "parent1" << "realChild1" << "grandChild1" << "realGrandChild1" << "child1" << "parent2" << "realChild2" << "grandChild2" << "realGrandChild2" << "child2"); - // Set a name filter that matches nothing -> only expanded folders remain. + // Set a name filter that matches nothing -> nothing will remain. m_model->setNameFilter("xyz"); - QCOMPARE(itemsInModel(), QStringList() << "parent1" << "realChild1" << "parent2" << "realChild2"); + QCOMPARE(itemsInModel(), QStringList()); QCOMPARE(itemsRemovedSpy.count(), 1); QList arguments = itemsRemovedSpy.takeFirst(); KItemRangeList itemRangeList = arguments.at(0).value(); - QCOMPARE(itemRangeList, KItemRangeList() << KItemRange(2, 3) << KItemRange(7, 3)); + QCOMPARE(itemRangeList, KItemRangeList() << KItemRange(0, 10)); + + // Set a name filter that matches only "realChild". Their prarents should still show. + m_model->setNameFilter("realChild"); + QCOMPARE(itemsInModel(), QStringList() << "parent1" << "realChild1" << "parent2" << "realChild2"); + QCOMPARE(itemsRemovedSpy.count(), 0); // nothing was removed, itemsRemovedSpy will not be called this time // Collapse "parent1". m_model->setExpanded(0, false);