Changeset View
Changeset View
Standalone View
Standalone View
plugins/execute/nativeappjob.cpp
| Show All 13 Lines | 1 | /* This file is part of KDevelop | |||
|---|---|---|---|---|---|
| 14 | You should have received a copy of the GNU Library General Public License | 14 | You should have received a copy of the GNU Library General Public License | ||
| 15 | along with this library; see the file COPYING.LIB. If not, write to | 15 | along with this library; see the file COPYING.LIB. If not, write to | ||
| 16 | the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, | 16 | the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, | ||
| 17 | Boston, MA 02110-1301, USA. | 17 | Boston, MA 02110-1301, USA. | ||
| 18 | */ | 18 | */ | ||
| 19 | 19 | | |||
| 20 | #include "nativeappjob.h" | 20 | #include "nativeappjob.h" | ||
| 21 | 21 | | |||
| 22 | #include <set> | ||||
| 23 | | ||||
| 22 | #include <QFileInfo> | 24 | #include <QFileInfo> | ||
| 23 | #include <QMessageBox> | 25 | #include <QMessageBox> | ||
| 24 | 26 | | |||
| 25 | #include <KConfigGroup> | 27 | #include <KConfigGroup> | ||
| 26 | #include <KLocalizedString> | 28 | #include <KLocalizedString> | ||
| 27 | #include <KShell> | 29 | #include <KShell> | ||
| 28 | #include <KSharedConfig> | 30 | #include <KSharedConfig> | ||
| 29 | 31 | | |||
| ▲ Show 20 Lines • Show All 98 Lines • ▼ Show 20 Line(s) | 128 | if (!job) { | |||
| 128 | if (!jobs.isEmpty()) | 130 | if (!jobs.isEmpty()) | ||
| 129 | job = jobs.first(); | 131 | job = jobs.first(); | ||
| 130 | } | 132 | } | ||
| 131 | return job; | 133 | return job; | ||
| 132 | } | 134 | } | ||
| 133 | 135 | | |||
| 134 | void NativeAppJob::start() | 136 | void NativeAppJob::start() | ||
| 135 | { | 137 | { | ||
| 136 | // we kill any execution of the configuration | | |||
| 137 | auto currentJobs = ICore::self()->runController()->currentJobs(); | 138 | auto currentJobs = ICore::self()->runController()->currentJobs(); | ||
| 139 | std::set<NativeAppJob*> ignoreJobs{this}; | ||||
mwolff: use QSet, or unordered_set - the order isn't needed and the hash-based containers are faster | |||||
| 138 | for (auto it = currentJobs.begin(); it != currentJobs.end();) { | 140 | for (auto it = currentJobs.begin(); it != currentJobs.end();) { | ||
| 139 | NativeAppJob* job = findNativeJob(*it); | 141 | NativeAppJob* job = findNativeJob(*it); | ||
| 140 | if (job && job != this && job->m_name == m_name) { | 142 | if (job && job->m_name == m_name && !ignoreJobs.count(job)) { | ||
I'd prefer QSet and then you could use the more expressive !contains here instead of !count mwolff: I'd prefer QSet and then you could use the more expressive `!contains` here instead of `!count` | |||||
| 141 | QMessageBox::StandardButton button = QMessageBox::question(nullptr, i18n("Job already running"), i18n("'%1' is already being executed. Should we kill the previous instance?", m_name)); | 143 | switch (QMessageBox::question(nullptr, i18n("Job already running"), i18n("'%1' is already being executed. Should we kill the previous instance?", m_name), QMessageBox::Yes | QMessageBox::No | QMessageBox::Cancel)) { | ||
please put the QMessageBox::question call into a separate line, and break it up. Save the return value in a temp var and switch over it then Also reword the message to '%1' is already being executed.. Then set proper texts on the button such that it's clear what they do: Yes -> Kill I'd also say that the default action should be cancel here, or maybe "start another", but not a destructive kill mwolff: please put the `QMessageBox::question` call into a separate line, and break it up. Save the… | |||||
| 142 | if (button != QMessageBox::No && ICore::self()->runController()->currentJobs().contains(*it)) { | 144 | case QMessageBox::Yes: // kill the running instance | ||
| 145 | if (ICore::self()->runController()->currentJobs().contains(*it)) { | ||||
why this line? you do this, to check whether the job is still alive? If so, store it in a QPointer and check its validity mwolff: why this line? you do this, to check whether the job is still alive? If so, store it in a… | |||||
| 143 | (*it)->kill(); | 146 | (*it)->kill(); | ||
| 144 | } | 147 | } | ||
| 148 | break; | ||||
| 149 | case QMessageBox::No: // simply start another job | ||||
| 150 | ignoreJobs.emplace(job); | ||||
| 151 | break; | ||||
| 152 | default: // cancel starting a new job | ||||
| 153 | kill(); | ||||
| 154 | return; | ||||
| 155 | } | ||||
| 145 | currentJobs = ICore::self()->runController()->currentJobs(); | 156 | currentJobs = ICore::self()->runController()->currentJobs(); | ||
| 146 | it = currentJobs.begin(); | 157 | it = currentJobs.begin(); | ||
| 147 | } else { | 158 | } else { | ||
| 148 | ++it; | 159 | ++it; | ||
mwolff: use a QVector please | |||||
| 149 | } | 160 | } | ||
| 150 | } | 161 | } | ||
| 151 | 162 | | |||
| 152 | OutputExecuteJob::start(); | 163 | OutputExecuteJob::start(); | ||
| 153 | } | 164 | } | ||
use QSet, or unordered_set - the order isn't needed and the hash-based containers are faster