Note: kcoreaddons, baloo, kservice/autotests, and kdelibs4support mention this key too, you can clean it all up ;)

Indeed, not used, and not useful.

BTW, the commit message refers to ModelTest, but that's the old modeltest.cpp, not the new QAbstractItemModelTester, right?

Well then you can just remove lessThan altogether, and call std::sort with only 2 arguments instead of 3 ;)

Done.

Heh, indeed.

In D7451#153015, @hindenburg wrote:

I've noticed that since this commit, I often accidentally drag-n-drop a tab outside the window such that it creates a new window.

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)

Yep, this makes sense these days (back when this was written, QUrl had no hashing function, and my first attempt at writing one was to call qHash(url.toString()) -- how horribly slow!!)

Unlike KFileItem, this function does no caching. I hope you do caching at the layer above then...

ping?

It should be easy to use lxr to see which KDE applications uses KFileItemListProperties::mimeType(), but OK, your solution is safer in case of third-party applications.

Even old OpenSuSE Leap 42.3 has ffmpeg 3.3.4, so this seems fine ;-)

Oops, thanks for the fix.

Still strange that "rss" or "rdf" anywhere in the path is enough, but I'm no rss expert... Looks good overall.

Found stuff on the way, but the commit itself is fine.

Nice

Starting to look good ;-)

Patch looks good, but please update the first line of the commit message so that it gives more context (think about the changelog, where the rest of the description won't be visible...)

Oh OK I had misunderstood your comment.

I requested it, for proper encapsulation

First line of commit log should have changelog in mind, so more context needed. Maybe rather something like "KIO::UDSEntry: switch to a single std::vector for more performance".

Looks good.

It helps if you actually add reviewers...

Makes sense, but why not do this in KFileItemListProperties::mimeType()?

Please add QStandardPaths::setTestModeEnabled(true) in initTestCase() so that your (broken, thank you WinE) locally defined mimetypes don't interfer with the test. Then "hardcoding" text/html will be fine again.

Thanks

I don't understand the "except" in your reply.

I like the replaceOrInsert idea, and the assert in insert... we might have to fix some kioslaves, but in general they have no good reason to insert twice for the same key.

In both cases the same fragility exists, in case one of the replacement strings contains one of the placeholders --- unless you use multi-arg, i.e. myString.arg(title, direction, m_warningIconData, ...)

In D12659#257292, @bruns wrote:

In D12659#257098, @dfaure wrote:

Thanks for that investigation. Interesting that linear search is faster than binary search in the same data structure... maybe the compiler optimizes it better? Did you profile V2 to find out where the time is spent, or do you have a better explanation?

Binary search has a (small) overhead, you can hardly beat linear search when the number of entries is small. When you use binary search, you pay when inserting - find the right position, probably move other items. When you do inserts one item a time, you have O(n^2) behavior.

Thanks for that investigation. Interesting that linear search is faster than binary search in the same data structure... maybe the compiler optimizes it better? Did you profile V2 to find out where the time is spent, or do you have a better explanation?
But even if both were equal performance-wise I'd favor linear search because sorted insertion is easy to get wrong - as this patch shows ;)

Thanks for the fixes, here are a few more nitpicks ;)

Nice! I remember that this was sorely missing when I was using that part :-)

Clearly we should port to the qt5 connect syntax...