D28020: New class ProcessLauncherJob in KIOGui

[David Edmundson]

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20200313/03af19f8/attachment-0001.html>