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 QApp->windows to find the > object from windowId Actually if D28016 <https://phabricator.kde.org/D28016> is approved, I can remove the windowId altogether. > davidedmundson wrote in processlauncherjob.h:111 > I don't like that this has to be available immediately after start() > > it means we can't make all the blocking calls for kiofuse/discrete > gpus/whatever async. > > which would be really nice for the future. > > can we have a signal > started(); > > and it's valid after that? > > or...alternatively have ProcessLaunchJob::finished() come after > processStarted, instead of when the process ends? and this is valid when the > job completes? From a plasma POV I just want to know if kate started ok, but > I don't care if it crashes later. This is an excellent point. This keeps bothering me too. One issue is that in order to use this from KRun I needed the PID immediately. But maybe we can add a blocking method (with QProcess::waitForStarted, no event loop there) specifically for KRun, until KF6. Re job completion, I agree that apps only care about starting the process. The idea of watching for exit code to detect non-existing executable was only a small optimization in 2000 (commit 15bd0141a63f244bdd6f9 <https://phabricator.kde.org/R446:15bd0141a63f244bdd6f9c134131303039985a65>), let's move this around and check before hand, it'll be much simpler. 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
