On Monday 26 August 2013 14:52:48 Vishesh Handa wrote:
> Hey David
> 
> I was trying to port kmimetypechooser away from KRun, so I started to look
> at the implementation of KRun. It seems to be somewhat strange -
> 
> bool KRun::runCommand(const QString& cmd, const QString &execName,
> const QString & iconName,
>                       QWidget* window, const QByteArray& asn, const QString&
> workingDirectory)
> {
>     //qDebug() << "runCommand " << cmd << "," << execName;
>     KProcess * proc = new KProcess;
>     proc->setShellCommand(cmd);
>     if (!workingDirectory.isEmpty()) {
>         proc->setWorkingDirectory(workingDirectory);
>     }
>     QString bin = binaryName(execName, true);
>     KService::Ptr service = KService::serviceByDesktopName(bin);
>     return runCommandInternal(cmd, service.data(),
>                               execName /*executable to check for in
> slotProcessExited*/,
>                               execName /*user-visible name*/,
>                               iconName, window, asn, workingDirectory);
> }
> 
> It seems that the KProcess is created and then not used at all. The git log
> doesn't seem to reveal much. Could you please take look?

In kdelibs 4.11 the first argument to runCommandInternal is "proc".

Martin Sandsmark changed this in ac2ade2766a4d in order to port to QProcess, 
by creating the QProcess further down the chain (in the ProcessRunner created 
by runCommandInternal).
But he left the KProcess code above by mistake; it can indeed be removed.
Also, the port to QProcess is broken, QProcess::start() doesn't support shell 
commands. Martin, please use KProcess again instead of QProcess, so that 
setShellCommand can still be used. We left KProcess in kcoreaddons after all, 
for this purpose.

-- 
David Faure, fa...@kde.org, http://www.davidfaure.fr
Working on KDE, in particular KDE Frameworks 5

_______________________________________________
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Reply via email to