Differential D8258

[Task Tools] Treat applications: and respective absolute path equal in launcherUrlsMatch
ClosedPublic
Actions

Authored by broulik on Oct 12 2017, 8:56 AM.

Details

Reviewers
hein
ngraham
mvourlakos
Group Reviewers
Plasma
Commits
R120:88dbb40ddede: [Task Tools] Treat applications: and respective absolute path equal in…
Summary

BUG: 385594

Test Plan

Having a file:///usr/share/applications/org.kde.dolphin.desktop launcher is no longer juggled around as I launch and quit Dolphin. Thanks Michail and Nate for pointing me to the right place (launcherPosition)

Diff Detail

Repository
R120 Plasma Workspace
Lint
Lint Skipped
Unit
Unit Tests Skipped
broulik created this revision.Oct 12 2017, 8:56 AM
Restricted Application added a project: Plasma. · View Herald TranscriptOct 12 2017, 8:56 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
broulik edited the test plan for this revision. (Show Details)Oct 12 2017, 8:57 AM
broulik updated this revision to Diff 20627.Oct 12 2017, 9:12 AM
  • Use same approach as appDataFromUrl (ie. KDesktopFile::isDesktopFile and then KDesktopFile fileName
davidedmundson added a subscriber: davidedmundson.Oct 13 2017, 10:17 AM
davidedmundson added inline comments.
libtaskmanager/tasktools.cpp
616

launcherUrslMatch is in a somewhat hot path that is called a lot.

If we have to do this, fine, but if there's an alternative; such as resolving the launcher to a KService (KService::serviceByMenuId) once in the launcher, that would be IMHO much much better.

Also menuId is built from the filename; (vfolder_menu.cpp:947) (prefix in that line is empty for anything non-supersuper-legacy)

Is there a reason we can't just compare filenames here?

broulik added inline comments.Oct 13 2017, 10:45 AM
libtaskmanager/tasktools.cpp
616

resolving the launcher to a KService (KService::serviceByMenuId) once in the launcher,

?

Is there a reason we can't just compare filenames here?

Don't know, I just looked at what the rest of the code dealing with applications: URLs does.

hein accepted this revision.Oct 13 2017, 5:44 PM
This revision is now accepted and ready to land.Oct 13 2017, 5:44 PM
hein added a comment.Oct 13 2017, 5:46 PM

Let's merge this for now to have a fix for 5.11.1. I'll look into speeding things up next week.

Closed by commit R120:88dbb40ddede: [Task Tools] Treat applications: and respective absolute path equal in… (authored by broulik). · Explain WhyOct 16 2017, 8:48 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mlibtaskmanager/tasktools.cpp (33 lines)

Diff 20627

View Options

libtaskmanager/tasktools.cpp

Loading...