D28201: Add KIO::CommandLauncherJob to replace KRun::runCommand

dfaure added a comment. In D28201#635026 , @broulik wrote: > I think we should also have a replacement job for `new KRun(url)` Yes, these launcher jobs are the building blocks for finally being able to turn KRun into a job (KIO::OpenUrlJo

D28201: Add KIO::CommandLauncherJob to replace KRun::runCommand

dfaure added inline comments. INLINE COMMENTS > commandlauncherjobtest.cpp:99 > + > +void CommandLauncherJobTest::doesNotFailOnNonExistingExecutable() > +{ See this unittest ;-) As the comment says, QProcess works since it manages to start /bin/sh. We don't want to wait until the application/p

D28201: Add KIO::CommandLauncherJob to replace KRun::runCommand

broulik added a comment. One thing I noticed, when I do `new CommandLaunchJob("somenonexistantbinary")` no error is raised and I just get console output `/bin/sh: 1: somenonexistantbinary: not found Though this was also the case of `runCommand`, so maybe not an issue. REPOSITORY

D28201: Add KIO::CommandLauncherJob to replace KRun::runCommand

dfaure added a task: T11549: KIO: remove unneeded QWidget dependencies to set parent windows or display errors. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D28201 To: dfaure, apol, davidedmundson, nicolasfella, vkrause, broulik Cc: ahmadsamir, kde-frameworks-devel, LeGas

D28201: Add KIO::CommandLauncherJob to replace KRun::runCommand

davidedmundson added a comment. T11549 REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D28201 To: dfaure, apol, davidedmundson, nicolasfella, vkrause, broulik Cc: ahmadsamir, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ng

D28201: Add KIO::CommandLauncherJob to replace KRun::runCommand

davidedmundson added a comment. > Do we have a kf6 workboard entry for this effort? T12163 REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D28201 To: dfaure, apol, davidedmundson, nicolasfella, vkrause, broulik Cc: ahmadsamir, kde

D28201: Add KIO::CommandLauncherJob to replace KRun::runCommand

broulik added a comment. Do we have a kf6 workboard entry for this effort? I think we should also have a replacement job for `new KRun(url)` and I'd love if we could split out the "default app for this mimetype" logic we (also?) have in `KFileItemActions` out into a dedicated public class.

D28201: Add KIO::CommandLauncherJob to replace KRun::runCommand

dfaure closed this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D28201 To: dfaure, apol, davidedmundson, nicolasfella, vkrause, broulik Cc: ahmadsamir, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D28201: Add KIO::CommandLauncherJob to replace KRun::runCommand

davidedmundson accepted this revision. REPOSITORY R241 KIO BRANCH 2020_03_commandlauncherjob REVISION DETAIL https://phabricator.kde.org/D28201 To: dfaure, apol, davidedmundson, nicolasfella, vkrause, broulik Cc: ahmadsamir, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraha

D28201: Add KIO::CommandLauncherJob to replace KRun::runCommand

dfaure updated this revision to Diff 78407. dfaure added a comment. Add setDesktopName(), excellent idea by davidedmundson. Fix API docs, fix copyright years, fix "do terminate" comment. REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28201?vs=78222&id=7840

D28201: Add KIO::CommandLauncherJob to replace KRun::runCommand

ahmadsamir added inline comments. INLINE COMMENTS > commandlauncherjobtest.cpp:3 > +This file is part of the KDE libraries > +Copyright (c) 2014, 2020 David Faure > + same. > commandlauncherjobtest.h:3 > +This file is part of the KDE libraries > +Copyright (c) 2014, 2020 David

D28201: Add KIO::CommandLauncherJob to replace KRun::runCommand

davidedmundson accepted this revision. davidedmundson added a comment. This revision is now accepted and ready to land. > Maybe the first one should be renamed to ApplicationLauncherJob, now that I found out it wouldn't be a good place for running commands (the setters are too different).

D28201: Add KIO::CommandLauncherJob to replace KRun::runCommand

dfaure added a comment. (technically, connecting to &KJob::result is missing in the dolphin port, but unlike ProcessLauncherJob, CommandLauncherJob is very unlikely to fail. QProcess will only fail to start if we're out of resources for forking or if `sh` can't be executed... we don't detect

D28201: Add KIO::CommandLauncherJob to replace KRun::runCommand

dfaure added a comment. Overview: KRun::runService/runApplication => ProcessLauncherJob KRun::runCommand => CommandLauncherJob new KRun => OpenUrlJob, not written yet Maybe the first one should be renamed to ApplicationLauncherJob, now that I found out it wouldn't be a good place

D28201: Add KIO::CommandLauncherJob to replace KRun::runCommand

dfaure added a comment. `Tools/Compare Files` still works in dolphin (it uses KRun::runCommand). http://www.davidfaure.fr/2020/dolphin.diff ports it to CommandLauncherJob, it still works afterwards. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D28201 To: dfaure,

D28201: Add KIO::CommandLauncherJob to replace KRun::runCommand

dfaure created this revision. dfaure added reviewers: apol, davidedmundson, nicolasfella, vkrause, broulik. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. dfaure requested review of this revision. REVISION SUMMARY - Ported runCommand to CommandLauncherJob