D13211: Enable comparing KFileItems by url

[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 overload.
  The case of the invalid item vs an invalid URL will be interesting too ;)

INLINE COMMENTS

> kfileitemtest.cpp:305
> +    // a KFileItem without url is considered < than anything.
> +    QVERIFY(nulFileItem < nulFileItem);
> +    QVERIFY(nulFileItem < fileItem);

I wonder if that should be true, though ;-)

I guess it should rather be false, so that a binary search (based on < only) can deduce that a null item is *equal* to another null item, rather than both being lower than the other, which would be nonsense.

> kfileitemtest.cpp:314
> +    QVERIFY(!(fileItem3 < fileItem));
> +    // This should be false as they are equal
> +    QVERIFY(!(fileItem < fileItem));

... exactly. I think the same reasoning applies to a null item.

> kfileitem.h:490
> +    /**
> +     * Returns true if other's URL is lexically greater than this url; otherwise returns false
> +     * @since 5.47

"this item's URL"

> kfileitem.h:496
> +    /**
> +     * Returns true if url other is lexically greater than this url; otherwise returns false
> +     * @since 5.47

"than this item's URL", then

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D13211

To: jtamate, dfaure
Cc: kde-frameworks-devel, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20180530/c286af7d/attachment.html>