Re: Comparing KFileItems

[Frank Reininghaus]

Hi David,

2012/10/21 David Faure:
On Thursday 18 October 2012 13:54:09 Frank Reininghaus wrote:
Obviously, KFileItems are only considered equal if their d pointers
are the same.

Yes, i.e. created from the same place.

But I agree, let's compare URLs too, since they are "unique identifiers" for
resources.

Sounds good.

There is a Dolphin bug where this plays a role and leads to constant
high CPU usage:

https://bugs.kde.org/show_bug.cgi?id=304986

Tricky one, well done with the investigation!

Thanks :-)

A KFileItem is basically "a URL with extra info associated". I suppose we use
KFileItem and not QUrl as the key in KFileItemModelRolesUpdater simply in
order to have this extra info available? But the QUrl is really the key, the
rest is associated data.

Well, the class gets the KFileItems from KFileItemModel, and the 
PreviewJob wants a KFileItemList, so it seems natural to use KFileItems 
in KFileItemModelRolesUpdater as well, rather than converting to KUrls 
and back.

Is the attached patch OK for 4.9 from your point of view?

Best regards,
Frank
diff --git a/kio/kio/kfileitem.cpp b/kio/kio/kfileitem.cpp
index 482c4b6..ad62246 100644
--- a/kio/kio/kfileitem.cpp
+++ b/kio/kio/kfileitem.cpp
@@ -1307,13 +1307,12 @@ bool KFileItem::cmp( const KFileItem & item ) const
 
 bool KFileItem::operator==(const KFileItem& other) const
 {
-    // is this enough?
-    return d == other.d;
+    return d == other.d || d->m_url == other.d->m_url;
 }
 
 bool KFileItem::operator!=(const KFileItem& other) const
 {
-    return d != other.d;
+    return d != other.d && d->m_url != other.d->m_url;
 }
 
 #ifndef KDE_NO_DEPRECATED
diff --git a/kio/tests/kfileitemtest.cpp b/kio/tests/kfileitemtest.cpp
index fa88af7..1ac5d98 100644
--- a/kio/tests/kfileitemtest.cpp
+++ b/kio/tests/kfileitemtest.cpp
@@ -108,11 +108,12 @@ void KFileItemTest::testDetach()
     fileItem2.mark();
     QVERIFY(fileItem2.isMarked());
     QVERIFY(!fileItem.isMarked());
-    QVERIFY(fileItem != fileItem2);
+    QVERIFY(fileItem == fileItem2);
 
     fileItem = fileItem2;
     QVERIFY(fileItem2.isMarked());
     QVERIFY(fileItem == fileItem2);
+    QVERIFY(!(fileItem != fileItem2));
 }
 
 void KFileItemTest::testBasic()
@@ -246,7 +247,8 @@ void KFileItemTest::testCmp()
 
     KFileItem fileItem(KFileItem::Unknown, KFileItem::Unknown, KUrl(file.fileName()), true /*on demand*/);
     KFileItem fileItem2(KFileItem::Unknown, KFileItem::Unknown, KUrl(file.fileName()), false);
-    QVERIFY(fileItem != fileItem2); // created independently so not 'equal'
+    QVERIFY(fileItem == fileItem2); // created independently, but still 'equal'
+    QVERIFY(!(fileItem != fileItem2));
     QVERIFY(fileItem.cmp(fileItem2));
 }