D28020: New class ProcessLauncherJob in KIOGui

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/D28020 To: dfaure, apol, davidedmundson, nicolasfella, vkrause, broulik Cc: jbbgameich, kde-frameworks-devel, LeGas

D28020: New class ProcessLauncherJob in KIOGui

nicolasfella added a comment. IIRC startup notification is still a big question mark in Wayland, but other Plasma devs know better REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D28020 To: dfaure, apol, davidedmundson, nicolasfella, vkrause, broulik Cc: jbbgameich, kde-

D28020: New class ProcessLauncherJob in KIOGui

dfaure added a comment. > Would it be possible to expose the finished signal of QProcess in KIO::ApplicationLauncherJob? By design the job finishes before the subprocess finishes (which could be in 3 days, if the user keeps the window open that long). Also this would only help you for

D28020: New class ProcessLauncherJob in KIOGui

jbbgameich added a comment. Sorry for using this diff to ask this question, I couldn't find you in kde-devel. Would it be possible to expose the finished signal of QProcess in KIO::ApplicationLauncherJob? I need something like this to close the startup feedback in the Plasma Mobile shell

D28020: New class ProcessLauncherJob in KIOGui

davidedmundson added a comment. In D28020#630185 , @dfaure wrote: > Ah there was still the WId question. I'll remove it before anyone starts using this API. > > We can always add a setter later if we want to (but if it's just for the unused

D28020: New class ProcessLauncherJob in KIOGui

dfaure added a comment. Ah there was still the WId question. I'll remove it before anyone starts using this API. We can always add a setter later if we want to (but if it's just for the unused setLaunchedBy, I'm not convinced...) REPOSITORY R241 KIO REVISION DETAIL https://phabrica

D28020: New class ProcessLauncherJob in KIOGui

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

D28020: New class ProcessLauncherJob in KIOGui

dfaure added inline comments. INLINE COMMENTS > davidedmundson wrote in kprocessrunner.cpp:194 > It's the DBus calls that come before start that I want to get async, not the > tiny bit between fork and the child process exec()'ing. > > Obviously we can do that piecemeal later, and it isn't a re

D28020: New class ProcessLauncherJob in KIOGui

davidedmundson accepted this revision. davidedmundson added inline comments. This revision is now accepted and ready to land. INLINE COMMENTS > kprocessrunner.cpp:194 > +{ > +return m_process->waitForStarted(); > +} It's the DBus calls that come before start that I want to get async, not the

D28020: New class ProcessLauncherJob in KIOGui

dfaure updated this revision to Diff 77682. dfaure added a comment. - Don't block in start(), make it fully async - Add waitForStarted() for KRun (with unittests) - Add test for non-existing executables, with and without kioexec - after making sure that the command isn't trivial, by sta

D28020: New class ProcessLauncherJob in KIOGui

dfaure planned changes to this revision. dfaure added inline comments. INLINE COMMENTS > davidedmundson wrote in kprocessrunner.cpp:39 > WId as an int is problematic for wayland. > > Can we do QWindow*? it'll allow adding support in future. > > For the compatibility path we can loop through QAp

D28020: New class ProcessLauncherJob in KIOGui

davidedmundson added a comment. From the POV of the task at hand, it's great. If we are making new public API I have some minor requests for things we want in future. INLINE COMMENTS > kprocessrunner.cpp:39 > + > +KProcessRunner::KProcessRunner(const KService::Ptr &service, const > QLi

D28020: New class ProcessLauncherJob in KIOGui

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 This is meant to replace KRun::runApplicatio