D13211: Enable comparing KFileItems by url

2018-06-13 Thread Mark Gaiser
markg added a comment. While this works, there is a newer en better way for it. It's new in C++14 and called "transparent compare". In "this" case it won't change the resulting code of how you compare, but it might be worth checking that out. Read this: https://www.fluentcpp.com/2017/

D13211: Enable comparing KFileItems by url

2018-06-13 Thread Jaime Torres Amate
This revision was automatically updated to reflect the committed changes. Closed by commit R241:fb3c94ed96c3: Enable comparing KFileItems by url (authored by jtamate). REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D13211?vs=36037&id=36112 REVISION DETAIL https:

D13211: Enable comparing KFileItems by url

2018-06-13 Thread David Faure
dfaure accepted this revision. This revision is now accepted and ready to land. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D13211 To: jtamate, dfaure Cc: bruns, kde-frameworks-devel, michaelh, ngraham

D13211: Enable comparing KFileItems by url

2018-06-12 Thread Jaime Torres Amate
jtamate updated this revision to Diff 36037. jtamate marked an inline comment as done. jtamate edited the summary of this revision. jtamate added a comment. Change @since to 5.48 REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D13211?vs=35303&id=36037 REVISION D

D13211: Enable comparing KFileItems by url

2018-06-01 Thread Jaime Torres Amate
jtamate updated this revision to Diff 35303. jtamate added a comment. Now passes the tests and its performance for non invalid items is not degraded too much (same +3ms inserting). REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D13211?vs=35192&id=35303 REVISIO

D13211: Enable comparing KFileItems by url

2018-05-30 Thread Jaime Torres Amate
jtamate updated this revision to Diff 35192. jtamate added a comment. Taken into account invalid Items created from invalid QUrls. REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D13211?vs=35190&id=35192 REVISION DETAIL https://phabricator.kde.org/D13211 AFFE

D13211: Enable comparing KFileItems by url

2018-05-30 Thread Jaime Torres Amate
jtamate updated this revision to Diff 35190. jtamate marked 4 inline comments as done. jtamate added a comment. Invalid items are not less than invalid items or invalid urls, they are not like -infinite. Added the tests comparing items with urls. Changed the descriptions. REPOSITORY R24

D13211: Enable comparing KFileItems by url

2018-05-30 Thread Jaime Torres Amate
jtamate updated this revision to Diff 35188. jtamate marked 3 inline comments as done. jtamate edited the test plan for this revision. jtamate added a comment. A KFileItem without url will be the lowest, even lower than itself. Created a new test. The comparison with the QUrl is for this

D13211: Enable comparing KFileItems by url

2018-05-30 Thread Stefan BrĂ¼ns
bruns added inline comments. INLINE COMMENTS > dfaure wrote in kfileitem.cpp:1241 > This isn't symmetric. operator< must have the property that a aren't both true, and that they are both false only if the items are equal. > > If `this` is a valid item and `other` is null, then this code says tha

D13211: Enable comparing KFileItems by url

2018-05-30 Thread David Faure
dfaure requested changes to this revision. dfaure added a comment. This revision now requires changes to proceed. A good example of how a unittest helps catching a bug :-) (and a good example of how code that I suggest isn't necessarily bugfree, haha) Missing: unittests for the < QUrl o

D13211: Enable comparing KFileItems by url

2018-05-30 Thread David Faure
dfaure requested changes to this revision. dfaure added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > kfileitem.cpp:1241 > +{ > +if (!d || !other.d) { > +return false; This isn't symmetric. operator< must have the property that a kfileitem.h:493 >

D13211: Enable comparing KFileItems by url

2018-05-30 Thread Jaime Torres Amate
jtamate created this revision. jtamate added a reviewer: dfaure. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: kde-frameworks-devel. jtamate requested review of this revision. REVISION SUMMARY Based on QUrl comparisons (available since Qt 5.4).

D13211: Enable comparing KFileItems by url

2018-05-30 Thread Jaime Torres Amate
jtamate added dependent revisions: D10742: get rid of the raw KFileItem pointers in KCoreDirListerCache, D12945: kcoredirlister lstItems benchmark. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D13211 To: jtamate, dfaure Cc: kde-frameworks-devel, michaelh, ngraham, bruns