Weigh matching services by relating data used in query to their menuids

Authored by hein on May 23 2018, 6:09 PM.



The overall mission of TaskTools::windowUrlFromMetadata is to use
various pieces of metadata to run KServiceTypeTrader queries and
get a list of matching services. Sometimes this will find more than
one service. So far we simply used whatever KServiceTypeTrader
returned first, but in some cases we can and should do better. The
included lengthy code comment names an example case.

In concert with D13058, this allows both the Linux-native and Wine-
installed (S|s)team.desktop files to coexist and their windows be
correctly mapped to the relevant .desktop file, by exploiting that
by their nature each case ends up with a different KService::menuId()
(that this is useful for differentiation is why the menuId differs,
after all).

This change looks like it introduces disgusting complexity at first,
but in some sense, trying to pick the most match-y out of the found
services instead of just randomly picking the first one makes sense.

Diff Detail

R120 Plasma Workspace
Automatic diff as part of commit; lint not applicable.
Automatic diff as part of commit; unit tests not applicable.
hein created this revision.May 23 2018, 6:09 PM
Restricted Application added a project: Plasma. · View Herald TranscriptMay 23 2018, 6:09 PM
hein requested review of this revision.May 23 2018, 6:09 PM
hein added a comment.May 23 2018, 6:13 PM

I'll add that KServiceTypeTrader doesn't offer any facilities for weighing. KService has a KService::initialPreference() but that seems only useful when sorting services for a MIME type by prioirty (and I'd guess it's maybe also used for KCM sorting?) and isn't applicable here either.

hein updated this revision to Diff 34740.May 23 2018, 6:14 PM

Optimize common case.

cfeck added a subscriber: cfeck.May 23 2018, 6:17 PM
cfeck added inline comments.

Can be removed? If not, sort correctly.

hein updated this revision to Diff 34741.May 23 2018, 6:18 PM

Remove forgotten QDebug include.

Thanks to @cfeck, who I now know is just as anal about
include collation as I am.

Fine in principle, surely adds less complexity than my weird rewriting stuff that also just attempted to fix Chrome :D


This comment makes it sound like this pretends to be a generic solution to a particular workaround. But you told me there's also Telegram and others being affected (fixed) by this, right?


Is there some nicer algo in place of this loop?


You're mutating the list (argument services is a non-const reference) and also returning it. Is this intentional?


isEmpty (unless consistent with the rest but it seems to be mixed already)

hein marked 3 inline comments as done.May 23 2018, 6:27 PM
hein added inline comments.

I started doing this for the Wine thing based on a user bug report. But for some reason I did wind up with two .desktop files for Telegram on my system, I think one from the Fedora package and one from ... a Flatpak? Their tarball? Who knows. In any case, this algo helps pick the right one there, too. The general idea of "if we find more than one service, the one with a menuId that's more closely related to the search term we found them with" seems to be better than "just use whatever KSTT returns first", considering KSTT does no sorting whatsoever.


You tell me? :)


Yes. It's just faster and less costly than actually sorting the list. This is a bit ugly, but it takes the stance of "this is a private anonymous function so it can't hurt any lib user, and if this sorting algo ever needs to be more sophisticated we'll fill it in then".


Will change.

hein updated this revision to Diff 34742.May 23 2018, 6:31 PM
hein marked 3 inline comments as done.
  • Consistently use isEmpty().
  • Make intro to comment less ambiguous.
hein updated this revision to Diff 34804.May 24 2018, 11:18 AM

Don't both mutate _and_ return.

davidedmundson accepted this revision.May 24 2018, 11:37 AM
This revision is now accepted and ready to land.May 24 2018, 11:37 AM
This revision was automatically updated to reflect the committed changes.