[IconApplet] Port KRun to ApplicationLauncherJob
ClosedPublic

Authored by ahmadsamir on May 12 2020, 5:46 PM.

Details

Test Plan

Open the Application Launcher, right click any app -> Add to Panel (Widget),
then click the icon on the panel, it should launch.

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.
ahmadsamir created this revision.May 12 2020, 5:46 PM
Restricted Application added a project: Plasma. · View Herald TranscriptMay 12 2020, 5:46 PM
ahmadsamir requested review of this revision.May 12 2020, 5:46 PM

(JFYI I just noticed that kde-cli-tools is broken and needs porting too)

broulik requested changes to this revision.May 13 2020, 7:03 AM
broulik added inline comments.
applets/icon/iconapplet.cpp
434–436

m_localPath is a *path* so you want serviceByDesktopPath

435

Use KNotificationJobUiDelegate

This revision now requires changes to proceed.May 13 2020, 7:03 AM
ahmadsamir updated this revision to Diff 82714.May 13 2020, 8:37 AM

Use KNotificationJobUiDelegate

broulik requested changes to this revision.May 13 2020, 8:49 AM
This revision now requires changes to proceed.May 13 2020, 8:49 AM

I forgot to submit my reply to your inline comment....

applets/icon/iconapplet.cpp
434–436

I tried that first, and it doesn't work; those .desktop files are in a ~/.local/share/plasma_icons, ksycoca doesn't know about that location.

broulik added inline comments.May 15 2020, 2:18 PM
applets/icon/iconapplet.cpp
434–436

But why would serviceByStorageId work then?
Looks like this needs to be KService::Ptr(new KService(m_localPath)) then?

ahmadsamir updated this revision to Diff 82943.May 15 2020, 2:27 PM

Address comments

ahmadsamir added inline comments.May 15 2020, 2:27 PM
applets/icon/iconapplet.cpp
434–436

According to the kservice docs KService::serviceByStorageId() param is "the storage id or desktop-file path of the service", so I guess that's why it works.

broulik added inline comments.May 15 2020, 2:28 PM
applets/icon/iconapplet.cpp
434–436

but "desktop file path" is what we're doing. what I believe is happening that you're actually launching the original file, not the one the icon uses.

ahmadsamir added inline comments.May 15 2020, 2:36 PM
applets/icon/iconapplet.cpp
434–436

I tried with:

KService::Ptr service = KService::serviceByStorageId(m_localPath);
qDebug() << "entryPath" << service->entryPath();
KIO::ApplicationLauncherJob *job = new KIO::ApplicationLauncherJob(service);

and got:
entryPath "/home/ahmad/.local/share/plasma_icons/org.kde.dolphin.desktop"

dfaure accepted this revision.May 17 2020, 1:43 PM
dfaure added a subscriber: dfaure.

KServiceFactory::findServiceByStorageId falls back to "new KService" if it can't find the storageId in ksycoca. So yes it worked, but it's kind of pointless (and confusing) to call it just to go to the new KService fallback, better do that directly.

dfaure added inline comments.May 17 2020, 1:45 PM
applets/icon/iconapplet.cpp
52

You're not using that class, are you?

ahmadsamir updated this revision to Diff 83017.May 17 2020, 1:56 PM

Remove one include

dfaure accepted this revision.May 17 2020, 2:21 PM
This revision was not accepted when it landed; it landed in state Needs Review.May 18 2020, 12:08 PM
This revision was automatically updated to reflect the committed changes.