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/
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:
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
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
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
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
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
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
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
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
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
>
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).
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
13 matches
Mail list logo