Differential D13073

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

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

Details

Reviewers
broulik
davidedmundson
Commits
R120:664b90afcfd8: Weigh matching services by relating data used in query to their menuids
Summary

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

Repository
R120 Plasma Workspace
Lint
Automatic diff as part of commit; lint not applicable.
Unit
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.
libtaskmanager/tasktools.cpp
45

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.

broulik added a comment.May 23 2018, 6:22 PM

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

libtaskmanager/tasktools.cpp
227

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?

242

Is there some nicer algo in place of this loop?

244

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

372

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.
libtaskmanager/tasktools.cpp
227

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.

242

You tell me? :)

244

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".

372

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
Closed by commit R120:664b90afcfd8: Weigh matching services by relating data used in query to their menuids (authored by hein). · Explain WhyMay 24 2018, 11:41 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mlibtaskmanager/tasktools.cpp (71 lines)

Diff 34808

View Options

libtaskmanager/tasktools.cpp

Loading...