D28266: KPropertiesDialog: port KRun::run to ApplicationLauncherJob
dfaure closed this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D28266 To: dfaure, davidedmundson, broulik Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns
D28266: KPropertiesDialog: port KRun::run to ApplicationLauncherJob
dfaure added inline comments. INLINE COMMENTS > broulik wrote in applicationlauncherjob.h:50 > Please add this in a separate commit Done in https://commits.kde.org/kio/01a4b70803d5bc9144a7ca7aec12081b4356c638 (also in CommandLauncherJob) > broulik wrote in kpropertiesdialog.cpp:1458 > Call this after setting everything up? If you want. Makes no difference, the actual start of a KJob is always delayed. REPOSITORY R241 KIO BRANCH master REVISION DETAIL https://phabricator.kde.org/D28266 To: dfaure, davidedmundson, broulik Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns
D28266: KPropertiesDialog: port KRun::run to ApplicationLauncherJob
broulik accepted this revision.
broulik added a comment.
This revision is now accepted and ready to land.
Also +1 for porting to kservice rather than launching binary
INLINE COMMENTS
> applicationlauncherjob.h:50
> *
> + * For error handling, either connect to the result() signal, or for a
> simple messagebox on error,
> + * you can do
Please add this in a separate commit
> kpropertiesdialog.cpp:1458
> +job->setUrls({ properties->url() });
> +job->start();
> +auto *delegate = new KDialogJobUiDelegate;
Call this after setting everything up?
REPOSITORY
R241 KIO
BRANCH
master
REVISION DETAIL
https://phabricator.kde.org/D28266
To: dfaure, davidedmundson, broulik
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns
D28266: KPropertiesDialog: port KRun::run to ApplicationLauncherJob
dfaure added a comment. ping? REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D28266 To: dfaure, davidedmundson, broulik Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns
D28266: KPropertiesDialog: port KRun::run to ApplicationLauncherJob
dfaure added a reviewer: broulik. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D28266 To: dfaure, davidedmundson, broulik Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns
D28266: KPropertiesDialog: port KRun::run to ApplicationLauncherJob
dfaure created this revision. dfaure added a reviewer: davidedmundson. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. dfaure requested review of this revision. REVISION SUMMARY KRun::run(exec, urls) is somewhere in between the two jobs. Since we know the name of the desktop file, we might as well use ApplicationLauncherJob here. Otherwise we would probably need a bit of KShell escaping to turn a QUrl into a command-line arg. TEST PLAN With filelight installed, the button in the properties dialog works. With the .desktop file still installed but the executable renamed, an error message pops up as expected. REPOSITORY R241 KIO BRANCH master REVISION DETAIL https://phabricator.kde.org/D28266 AFFECTED FILES src/gui/applicationlauncherjob.h src/widgets/kpropertiesdialog.cpp To: dfaure, davidedmundson Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns
