[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-core-devel
Subject:    Re: Comparing KFileItems
From:       Frank Reininghaus <frank78ac () googlemail ! com>
Date:       2012-10-22 18:10:23
Message-ID: 50858C0F.9000001 () googlemail ! com
[Download RAW message or body]

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

["KFileItem.diff" (text/x-patch)]

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));
 }
 



[prev in list] [next in list] [prev in thread] [next in thread]