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 > QList<QUrl> &urls, WId windowId, > + KIO::ProcessLauncherJob::RunFlags flags, > const QString &suggestedFileName, const QByteArray &asn) 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 > processlauncherjob.h:111 > + */ > + qint64 pid() const; > + 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. 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
