From 90dd8977520bfce23ff6669809db1e9ecb5ec060 Mon Sep 17 00:00:00 2001 From: Frank Reininghaus Date: Fri, 15 Mar 2013 00:23:44 +0100 Subject: [PATCH] Improve handling of filtered items when folders are deleted/collapsed If an expanded folder with filtered children is collapsed or removed, and the parent-child relationship cannot be determined by parsing the URLs, this patch makes sure that the filtered children do not reappear when the filter bar is cleared. REVIEW: 109455 --- src/kitemviews/kfileitemmodel.cpp | 130 ++++++++++++------------------ src/kitemviews/kfileitemmodel.h | 6 ++ src/tests/kfileitemmodeltest.cpp | 93 +++++++++++++++++++++ 3 files changed, 149 insertions(+), 80 deletions(-) diff --git a/src/kitemviews/kfileitemmodel.cpp b/src/kitemviews/kfileitemmodel.cpp index 40cd75094..5d1a1968a 100644 --- a/src/kitemviews/kfileitemmodel.cpp +++ b/src/kitemviews/kfileitemmodel.cpp @@ -1,22 +1,23 @@ -/*************************************************************************** - * Copyright (C) 2011 by Peter Penz * - * Copyright (C) 2013 by Frank Reininghaus * - * * - * This program is free software; you can redistribute it and/or modify * - * it under the terms of the GNU General Public License as published by * - * the Free Software Foundation; either version 2 of the License, or * - * (at your option) any later version. * - * * - * This program is distributed in the hope that it will be useful, * - * but WITHOUT ANY WARRANTY; without even the implied warranty of * - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the * - * GNU General Public License for more details. * - * * - * You should have received a copy of the GNU General Public License * - * along with this program; if not, write to the * - * Free Software Foundation, Inc., * - * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA * - ***************************************************************************/ +/***************************************************************************** + * Copyright (C) 2011 by Peter Penz * + * Copyright (C) 2013 by Frank Reininghaus * + * Copyright (C) 2013 by Emmanuel Pescosta * + * * + * This program is free software; you can redistribute it and/or modify * + * it under the terms of the GNU General Public License as published by * + * the Free Software Foundation; either version 2 of the License, or * + * (at your option) any later version. * + * * + * This program is distributed in the hope that it will be useful, * + * but WITHOUT ANY WARRANTY; without even the implied warranty of * + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the * + * GNU General Public License for more details. * + * * + * You should have received a copy of the GNU General Public License * + * along with this program; if not, write to the * + * Free Software Foundation, Inc., * + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA * + *****************************************************************************/ #include "kfileitemmodel.h" @@ -421,7 +422,8 @@ bool KFileItemModel::setExpanded(int index, bool expanded) return false; } - const KUrl url = m_itemData.at(index)->item.url(); + const KFileItem item = m_itemData.at(index)->item; + const KUrl url = item.url(); if (expanded) { m_expandedDirs.insert(url); m_dirLister->openUrl(url, KDirLister::Keep); @@ -429,38 +431,10 @@ bool KFileItemModel::setExpanded(int index, bool expanded) m_expandedDirs.remove(url); m_dirLister->stop(url); + removeFilteredChildren(KFileItemList() << item); - KFileItemList itemsToRemove; - const int expandedParentsCount = data(index)["expandedParentsCount"].toInt(); - ++index; - while (index < count() && data(index)["expandedParentsCount"].toInt() > expandedParentsCount) { - itemsToRemove.append(m_itemData.at(index)->item); - ++index; - } - - QSet urlsToRemove; - urlsToRemove.reserve(itemsToRemove.count() + 1); - urlsToRemove.insert(url); - foreach (const KFileItem& item, itemsToRemove) { - KUrl url = item.url(); - url.adjustPath(KUrl::RemoveTrailingSlash); - urlsToRemove.insert(url); - } - - QHash::iterator it = m_filteredItems.begin(); - while (it != m_filteredItems.end()) { - const KUrl url = it.key().url(); - KUrl parentUrl = url.upUrl(); - parentUrl.adjustPath(KUrl::RemoveTrailingSlash); - - if (urlsToRemove.contains(parentUrl)) { - delete it.value(); - it = m_filteredItems.erase(it); - } else { - ++it; - } - } - + const KFileItemList itemsToRemove = childItems(item); + removeFilteredChildren(itemsToRemove); removeItems(itemsToRemove, DeleteItemData); } @@ -598,6 +572,30 @@ void KFileItemModel::applyFilters() insertItems(newVisibleItems); } +void KFileItemModel::removeFilteredChildren(const KFileItemList& parentsList) +{ + if (m_filteredItems.isEmpty()) { + return; + } + + // First, we put the parent items into a set to provide fast lookup + // while iterating over m_filteredItems and prevent quadratic + // complexity if there are N parents and N filtered items. + const QSet parents = parentsList.toSet(); + + QHash::iterator it = m_filteredItems.begin(); + while (it != m_filteredItems.end()) { + const ItemData* parent = it.value()->parent; + + if (parent && parents.contains(parent->item)) { + delete it.value(); + it = m_filteredItems.erase(it); + } else { + ++it; + } + } +} + QList KFileItemModel::rolesInformation() { static QList rolesInfo; @@ -826,35 +824,7 @@ void KFileItemModel::slotItemsDeleted(const KFileItemList& items) } if (m_requestRole[ExpandedParentsCountRole]) { - // Remove all filtered children of deleted items. First, we put the - // deleted URLs into a set to provide fast lookup while iterating - // over m_filteredItems and prevent quadratic complexity if there - // are N removed items and N filtered items. - // - // TODO: This does currently *not* work if the parent-child - // relationships can not be determined just by using KUrl::upUrl(). - // This is the case, e.g., when browsing smb:/. - QSet urlsToRemove; - urlsToRemove.reserve(itemsToRemove.count()); - foreach (const KFileItem& item, itemsToRemove) { - KUrl url = item.url(); - url.adjustPath(KUrl::RemoveTrailingSlash); - urlsToRemove.insert(url); - } - - QHash::iterator it = m_filteredItems.begin(); - while (it != m_filteredItems.end()) { - const KUrl url = it.key().url(); - KUrl parentUrl = url.upUrl(); - parentUrl.adjustPath(KUrl::RemoveTrailingSlash); - - if (urlsToRemove.contains(parentUrl)) { - delete it.value(); - it = m_filteredItems.erase(it); - } else { - ++it; - } - } + removeFilteredChildren(itemsToRemove); } } diff --git a/src/kitemviews/kfileitemmodel.h b/src/kitemviews/kfileitemmodel.h index 85a8d0c0d..1d837cb2a 100644 --- a/src/kitemviews/kfileitemmodel.h +++ b/src/kitemviews/kfileitemmodel.h @@ -396,6 +396,12 @@ private: */ void applyFilters(); + /** + * Removes filtered items whose expanded parents have been deleted + * or collapsed via setExpanded(parentIndex, false). + */ + void removeFilteredChildren(const KFileItemList& parentsList); + /** * Maps the QByteArray-roles to RoleTypes and provides translation- and * group-contexts. diff --git a/src/tests/kfileitemmodeltest.cpp b/src/tests/kfileitemmodeltest.cpp index cd2e8d777..484ddee11 100644 --- a/src/tests/kfileitemmodeltest.cpp +++ b/src/tests/kfileitemmodeltest.cpp @@ -81,6 +81,7 @@ private slots: void testRemoveHiddenItems(); void collapseParentOfHiddenItems(); void removeParentOfHiddenItems(); + void testGeneralParentChildRelationships(); private: QStringList itemsInModel() const; @@ -1004,6 +1005,98 @@ void KFileItemModelTest::removeParentOfHiddenItems() QCOMPARE(itemsInModel(), QStringList() << "a" << "1"); } +/** + * Create a tree structure where parent-child relationships can not be + * determined by parsing the URLs, and verify that KFileItemModel + * handles them correctly. + */ +void KFileItemModelTest::testGeneralParentChildRelationships() +{ + QSet modelRoles = m_model->roles(); + modelRoles << "isExpanded" << "isExpandable" << "expandedParentsCount"; + m_model->setRoles(modelRoles); + + QStringList files; + files << "parent1/realChild1/realGrandChild1" << "parent2/realChild2/realGrandChild2"; + m_testDir->createFiles(files); + + m_model->loadDirectory(m_testDir->url()); + QVERIFY(QTest::kWaitForSignal(m_model, SIGNAL(itemsInserted(KItemRangeList)), DefaultTimeout)); + QCOMPARE(itemsInModel(), QStringList() << "parent1" << "parent2"); + + // Expand all folders. + m_model->setExpanded(0, true); + QVERIFY(QTest::kWaitForSignal(m_model, SIGNAL(itemsInserted(KItemRangeList)), DefaultTimeout)); + QCOMPARE(itemsInModel(), QStringList() << "parent1" << "realChild1" << "parent2"); + + m_model->setExpanded(1, true); + QVERIFY(QTest::kWaitForSignal(m_model, SIGNAL(itemsInserted(KItemRangeList)), DefaultTimeout)); + QCOMPARE(itemsInModel(), QStringList() << "parent1" << "realChild1" << "realGrandChild1" << "parent2"); + + m_model->setExpanded(3, true); + QVERIFY(QTest::kWaitForSignal(m_model, SIGNAL(itemsInserted(KItemRangeList)), DefaultTimeout)); + QCOMPARE(itemsInModel(), QStringList() << "parent1" << "realChild1" << "realGrandChild1" << "parent2" << "realChild2"); + + m_model->setExpanded(4, true); + QVERIFY(QTest::kWaitForSignal(m_model, SIGNAL(itemsInserted(KItemRangeList)), DefaultTimeout)); + QCOMPARE(itemsInModel(), QStringList() << "parent1" << "realChild1" << "realGrandChild1" << "parent2" << "realChild2" << "realGrandChild2"); + + // Add some more children and grand-children. + const KUrl parent1 = m_model->fileItem(0).url(); + const KUrl parent2 = m_model->fileItem(3).url(); + const KUrl realChild1 = m_model->fileItem(1).url(); + const KUrl realChild2 = m_model->fileItem(4).url(); + + m_model->slotItemsAdded(parent1, KFileItemList() << KFileItem(KUrl("child1"), QString(), KFileItem::Unknown)); + m_model->slotCompleted(); + QCOMPARE(itemsInModel(), QStringList() << "parent1" << "realChild1" << "realGrandChild1" << "child1" << "parent2" << "realChild2" << "realGrandChild2"); + + m_model->slotItemsAdded(parent2, KFileItemList() << KFileItem(KUrl("child2"), QString(), KFileItem::Unknown)); + m_model->slotCompleted(); + QCOMPARE(itemsInModel(), QStringList() << "parent1" << "realChild1" << "realGrandChild1" << "child1" << "parent2" << "realChild2" << "realGrandChild2" << "child2"); + + m_model->slotItemsAdded(realChild1, KFileItemList() << KFileItem(KUrl("grandChild1"), QString(), KFileItem::Unknown)); + m_model->slotCompleted(); + QCOMPARE(itemsInModel(), QStringList() << "parent1" << "realChild1" << "grandChild1" << "realGrandChild1" << "child1" << "parent2" << "realChild2" << "realGrandChild2" << "child2"); + + m_model->slotItemsAdded(realChild1, KFileItemList() << KFileItem(KUrl("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(KUrl("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. + QSignalSpy itemsRemovedSpy(m_model, SIGNAL(itemsRemoved(KItemRangeList))); + m_model->setNameFilter("xyz"); + QCOMPARE(itemsInModel(), QStringList() << "parent1" << "realChild1" << "parent2" << "realChild2"); + QCOMPARE(itemsRemovedSpy.count(), 1); + QList arguments = itemsRemovedSpy.takeFirst(); + KItemRangeList itemRangeList = arguments.at(0).value(); + QCOMPARE(itemRangeList, KItemRangeList() << KItemRange(2, 3) << KItemRange(7, 3)); + + // Collapse "parent1". + m_model->setExpanded(0, false); + QCOMPARE(itemsInModel(), QStringList() << "parent1" << "parent2" << "realChild2"); + QCOMPARE(itemsRemovedSpy.count(), 1); + arguments = itemsRemovedSpy.takeFirst(); + itemRangeList = arguments.at(0).value(); + QCOMPARE(itemRangeList, KItemRangeList() << KItemRange(1, 1)); + + // Remove "parent2". + m_model->slotItemsDeleted(KFileItemList() << m_model->fileItem(1)); + QCOMPARE(itemsInModel(), QStringList() << "parent1"); + QCOMPARE(itemsRemovedSpy.count(), 1); + arguments = itemsRemovedSpy.takeFirst(); + itemRangeList = arguments.at(0).value(); + QCOMPARE(itemRangeList, KItemRangeList() << KItemRange(1, 2)); + + // Clear filter, verify that no items reappear. + m_model->setNameFilter(QString()); + QCOMPARE(itemsInModel(), QStringList() << "parent1"); +} + QStringList KFileItemModelTest::itemsInModel() const { QStringList items; -- 2.47.3