D8199: Allow abortion of starting another NativeAppJob

[Milian Wolff]

mwolff added a comment.


  looking at the old code, I wonder why it did what it did... Most notably, it refetched the current jobs after the message box. This looks intentional and is now gone. I mean sure, while a dialog is shown, anything could happen, including running another job... Do we need to take care of this? Does anyone remember why it was done in this way? Does the git history show up anything?
  
  If we don't need that anymore, can we get rid of the loop altogether? I mean either we want to kill all jobs, or none. Showing one dialog per job is confusing as there is not indication which job we are going to kill out of multiple ones...

INLINE COMMENTS

> nativeappjob.cpp:138
>  {
> -    // we kill any execution of the configuration
> -    auto currentJobs = ICore::self()->runController()->currentJobs();
> -    for (auto it = currentJobs.begin(); it != currentJobs.end();) {
> -        NativeAppJob* job = findNativeJob(*it);
> -        if (job && job != this && job->m_name == m_name) {
> -            QMessageBox::StandardButton button = QMessageBox::question(nullptr, i18n("Job already running"), i18n("'%1' is already being executed. Should we kill the previous instance?", m_name));
> -            if (button != QMessageBox::No && ICore::self()->runController()->currentJobs().contains(*it)) {
> -                (*it)->kill();
> -            }
> -            currentJobs = ICore::self()->runController()->currentJobs();
> -            it = currentJobs.begin();
> -        } else {
> -            ++it;
> +    QList<QPointer<NativeAppJob> > currentJobs;
> +    // collect running instances of the same type

use a QVector please

REPOSITORY
  R32 KDevelop

REVISION DETAIL
  https://phabricator.kde.org/D8199

To: croick, #kdevelop, mwolff
Cc: mwolff, kdevelop-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20171009/749681ed/attachment.html>