Changeset View
Standalone View
src/core/deletejob.cpp
Show All 33 Lines | |||||
34 | #include <klocalizedstring.h> | 34 | #include <klocalizedstring.h> | ||
35 | #include <kio/jobuidelegatefactory.h> | 35 | #include <kio/jobuidelegatefactory.h> | ||
36 | 36 | | |||
37 | #include <QTimer> | 37 | #include <QTimer> | ||
38 | #include <QFile> | 38 | #include <QFile> | ||
39 | #include <QFileInfo> | 39 | #include <QFileInfo> | ||
40 | #include <QDir> | 40 | #include <QDir> | ||
41 | #include <QPointer> | 41 | #include <QPointer> | ||
42 | #include <QThread> | ||||
43 | #include <QMetaObject> | ||||
42 | 44 | | |||
43 | #include "job_p.h" | 45 | #include "job_p.h" | ||
44 | 46 | | |||
45 | extern bool kio_resolve_local_urls; // from copyjob.cpp, abused here to save a symbol. | 47 | extern bool kio_resolve_local_urls; // from copyjob.cpp, abused here to save a symbol. | ||
46 | 48 | | |||
47 | static bool isHttpProtocol(const QString &protocol) | 49 | static bool isHttpProtocol(const QString &protocol) | ||
48 | { | 50 | { | ||
49 | return (protocol.startsWith(QLatin1String("webdav"), Qt::CaseInsensitive) || | 51 | return (protocol.startsWith(QLatin1String("webdav"), Qt::CaseInsensitive) || | ||
50 | protocol.startsWith(QLatin1String("http"), Qt::CaseInsensitive)); | 52 | protocol.startsWith(QLatin1String("http"), Qt::CaseInsensitive)); | ||
51 | } | 53 | } | ||
52 | 54 | | |||
53 | namespace KIO | 55 | namespace KIO | ||
54 | { | 56 | { | ||
55 | enum DeleteJobState { | 57 | enum DeleteJobState { | ||
56 | DELETEJOB_STATE_STATING, | 58 | DELETEJOB_STATE_STATING, | ||
57 | DELETEJOB_STATE_DELETING_FILES, | 59 | DELETEJOB_STATE_DELETING_FILES, | ||
58 | DELETEJOB_STATE_DELETING_DIRS | 60 | DELETEJOB_STATE_DELETING_DIRS | ||
59 | }; | 61 | }; | ||
60 | 62 | | |||
61 | /* | 63 | class DeleteJobIOWorker : public QObject { | ||
62 | static const char* const s_states[] = { | 64 | Q_OBJECT | ||
63 | "DELETEJOB_STATE_STATING", | 65 | | ||
64 | "DELETEJOB_STATE_DELETING_FILES", | 66 | Q_SIGNALS: | ||
dfaure: can be removed | |||||
65 | "DELETEJOB_STATE_DELETING_DIRS" | 67 | void rmfileResult(bool succeeded, const QUrl& url, bool isLink); | ||
dfaure: "const bool" doesn't do anything in a signal, I suggest removing the const. | |||||
I am hesitant to remove const QUrl& url from this signal, it could save some space in message passing. meven: I am hesitant to remove `const QUrl& url` from this signal, it could save some space in message… | |||||
Seems negligible to me. If you remove the argument, you'll have to "reconstruct" the URL from files.first()/symlinks.first()? Or [ab]use m_currentURL? dfaure: Seems negligible to me. If you remove the argument, you'll have to "reconstruct" the URL from… | |||||
With my code change m_currentUrl is always equal to this url just before calling rmfile/rmdir so yes it would be about [ab]using m_currentURL. meven: With my code change `m_currentUrl` is always equal to this `url` just before calling… | |||||
66 | }; | 68 | void rmddirResult(bool succeeded, const QUrl& url); | ||
69 | | ||||
70 | public Q_SLOTS: | ||||
71 | | ||||
72 | /** | ||||
73 | * Deletes the file @p url points to | ||||
Missing "url" after @p (which introduces the name of an argument). Not that we're going to run doxygen here, but well ;) dfaure: Missing "url" after @p (which introduces the name of an argument). Not that we're going to run… | |||||
74 | * The file must be a LocalFile | ||||
67 | */ | 75 | */ | ||
76 | void rmfile(const QUrl& url, bool isLink){ | ||||
(here it technically does something (guarantees that it's not modified in the implementation), but that's quite unusual in Qt/KDE code; your choice) dfaure: (here it technically does something (guarantees that it's not modified in the implementation)… | |||||
dfaure: missing space before '{', sorry I missed it until now. | |||||
77 | emit rmfileResult(QFile::remove(url.toLocalFile()), url, isLink); | ||||
78 | } | ||||
79 | | ||||
80 | /** | ||||
81 | * Deletes the directory @p url points to | ||||
82 | * The directory must be a LocalFile | ||||
83 | */ | ||||
84 | void rmdir(const QUrl& url) { | ||||
85 | emit rmddirResult(QDir().rmdir(url.toLocalFile()), url); | ||||
86 | } | ||||
87 | }; | ||||
68 | 88 | | |||
69 | class DeleteJobPrivate: public KIO::JobPrivate | 89 | class DeleteJobPrivate: public KIO::JobPrivate | ||
70 | { | 90 | { | ||
71 | public: | 91 | public: | ||
72 | explicit DeleteJobPrivate(const QList<QUrl> &src) | 92 | explicit DeleteJobPrivate(const QList<QUrl> &src) | ||
73 | : state(DELETEJOB_STATE_STATING) | 93 | : state(DELETEJOB_STATE_STATING) | ||
74 | , m_processedFiles(0) | 94 | , m_processedFiles(0) | ||
75 | , m_processedDirs(0) | 95 | , m_processedDirs(0) | ||
76 | , m_totalFilesDirs(0) | 96 | , m_totalFilesDirs(0) | ||
77 | , m_srcList(src) | 97 | , m_srcList(src) | ||
78 | , m_currentStat(m_srcList.begin()) | 98 | , m_currentStat(m_srcList.begin()) | ||
79 | , m_reportTimer(nullptr) | 99 | , m_reportTimer(nullptr) | ||
100 | , m_ioworker(nullptr) | ||||
funny that you use the ctor init list for this one, and not a default value at declaration time, like you did for m_thread :-) dfaure: funny that you use the ctor init list for this one, and not a default value at declaration time… | |||||
80 | { | 101 | { | ||
81 | } | 102 | } | ||
82 | DeleteJobState state; | 103 | DeleteJobState state; | ||
83 | int m_processedFiles; | 104 | int m_processedFiles; | ||
84 | int m_processedDirs; | 105 | int m_processedDirs; | ||
85 | int m_totalFilesDirs; | 106 | int m_totalFilesDirs; | ||
86 | QUrl m_currentURL; | 107 | QUrl m_currentURL; | ||
87 | QList<QUrl> files; | 108 | QList<QUrl> files; | ||
88 | QList<QUrl> symlinks; | 109 | QList<QUrl> symlinks; | ||
89 | QList<QUrl> dirs; | 110 | QList<QUrl> dirs; | ||
90 | QList<QUrl> m_srcList; | 111 | QList<QUrl> m_srcList; | ||
91 | QList<QUrl>::iterator m_currentStat; | 112 | QList<QUrl>::iterator m_currentStat; | ||
92 | QSet<QString> m_parentDirs; | 113 | QSet<QString> m_parentDirs; | ||
93 | QTimer *m_reportTimer; | 114 | QTimer *m_reportTimer; | ||
115 | DeleteJobIOWorker *m_ioworker; | ||||
116 | QThread *m_thread = nullptr; | ||||
94 | 117 | | |||
95 | void statNextSrc(); | 118 | void statNextSrc(); | ||
119 | DeleteJobIOWorker* worker(); | ||||
96 | void currentSourceStated(bool isDir, bool isLink); | 120 | void currentSourceStated(bool isDir, bool isLink); | ||
97 | void finishedStatPhase(); | 121 | void finishedStatPhase(); | ||
98 | void deleteNextFile(); | 122 | void deleteNextFile(); | ||
99 | void deleteNextDir(); | 123 | void deleteNextDir(); | ||
100 | void restoreDirWatch() const; | 124 | void restoreDirWatch() const; | ||
101 | void slotReport(); | 125 | void slotReport(); | ||
102 | void slotStart(); | 126 | void slotStart(); | ||
103 | void slotEntries(KIO::Job *, const KIO::UDSEntryList &list); | 127 | void slotEntries(KIO::Job *, const KIO::UDSEntryList &list); | ||
104 | 128 | | |||
129 | | ||||
130 | /// Callback of worker rmfile | ||||
131 | void rmFileResult(bool result, const QUrl& url, bool isLink); | ||||
Now we are at it, isLink could become m_curentURLIsLink, but isLink being a bool, it is much cheaper to move around threads, so we don't have perf incentive to do it as we had with a QString. meven: Now we are at it, isLink could become m_curentURLIsLink, but isLink being a bool, it is much… | |||||
132 | /// Callback of worker rmdir | ||||
133 | void rmdirResult(bool result, const QUrl& url); | ||||
dfaure: no space after method name (same on next line) | |||||
134 | void deleteFileUsingJob(const QUrl& url, bool isLink); | ||||
135 | void deleteDirUsingJob(const QUrl& url); | ||||
136 | | ||||
137 | ~DeleteJobPrivate(); | ||||
138 | | ||||
105 | Q_DECLARE_PUBLIC(DeleteJob) | 139 | Q_DECLARE_PUBLIC(DeleteJob) | ||
106 | 140 | | |||
107 | static inline DeleteJob *newJob(const QList<QUrl> &src, JobFlags flags) | 141 | static inline DeleteJob *newJob(const QList<QUrl> &src, JobFlags flags) | ||
108 | { | 142 | { | ||
109 | DeleteJob *job = new DeleteJob(*new DeleteJobPrivate(src)); | 143 | DeleteJob *job = new DeleteJob(*new DeleteJobPrivate(src)); | ||
110 | job->setUiDelegate(KIO::createDefaultJobUiDelegate()); | 144 | job->setUiDelegate(KIO::createDefaultJobUiDelegate()); | ||
111 | if (!(flags & HideProgressInfo)) { | 145 | if (!(flags & HideProgressInfo)) { | ||
112 | KIO::getJobTracker()->registerJob(job); | 146 | KIO::getJobTracker()->registerJob(job); | ||
Show All 20 Lines | 162 | { | |||
133 | 167 | | |||
134 | QTimer::singleShot(0, this, SLOT(slotStart())); | 168 | QTimer::singleShot(0, this, SLOT(slotStart())); | ||
135 | } | 169 | } | ||
136 | 170 | | |||
137 | DeleteJob::~DeleteJob() | 171 | DeleteJob::~DeleteJob() | ||
138 | { | 172 | { | ||
139 | } | 173 | } | ||
140 | 174 | | |||
175 | DeleteJobPrivate::~DeleteJobPrivate() | ||||
176 | { | ||||
177 | if (m_thread) { | ||||
178 | m_thread->quit(); | ||||
179 | m_thread->wait(); | ||||
180 | } | ||||
181 | } | ||||
182 | | ||||
141 | QList<QUrl> DeleteJob::urls() const | 183 | QList<QUrl> DeleteJob::urls() const | ||
142 | { | 184 | { | ||
143 | return d_func()->m_srcList; | 185 | return d_func()->m_srcList; | ||
144 | } | 186 | } | ||
145 | 187 | | |||
146 | void DeleteJobPrivate::slotStart() | 188 | void DeleteJobPrivate::slotStart() | ||
147 | { | 189 | { | ||
148 | statNextSrc(); | 190 | statNextSrc(); | ||
149 | } | 191 | } | ||
150 | 192 | | |||
I would remove the "Private" from the name, it makes one think that there is a corresponding public class somewhere. dfaure: I would remove the "Private" from the name, it makes one think that there is a corresponding… | |||||
One problem left: creating the worker and its thread is done even if we're deleting only remote URLs. This is a bit wasteful. Suggestion: create a worker() method which does all this (everything that you added to this method) on demand, if m_ioworker is null. dfaure: One problem left: creating the worker and its thread is done even if we're deleting only remote… | |||||
193 | DeleteJobIOWorker* DeleteJobPrivate::worker() | ||||
194 | { | ||||
You're using the 4-args connect, which is static, so m_thread.connect is confusing/wrong. This should be QObject::connect instead. dfaure: You're using the 4-args connect, which is static, so `m_thread.connect` is confusing/wrong. | |||||
195 | Q_Q(DeleteJob); | ||||
Prefer QObject::connect(4 args), the receiver is missing and could be q_func() for extra safety (so that the connect is broken if the DeleteJob is deleted, even if m_ioworker is leaked for some reason). dfaure: Prefer QObject::connect(4 args), the receiver is missing and could be q_func() for extra safety… | |||||
196 | | ||||
197 | if (m_thread == nullptr) { | ||||
nitpick: it would be more expected to do if (!m_ioworker) { here, since m_ioworker is what we're creating on demand. m_thread is just an implementation detail. dfaure: nitpick: it would be more expected to do `if (!m_ioworker) {` here, since m_ioworker is what… | |||||
198 | m_thread = new QThread(); | ||||
199 | | ||||
200 | m_ioworker = new DeleteJobIOWorker; | ||||
201 | m_ioworker->moveToThread(m_thread); | ||||
202 | QObject::connect(m_thread, &QThread::finished, m_ioworker, &QObject::deleteLater); | ||||
203 | QObject::connect(m_ioworker, &DeleteJobIOWorker::rmfileResult, q, [=](bool result, const QUrl &url, bool isLink){ | ||||
204 | this->rmFileResult(result, url, isLink); | ||||
205 | }); | ||||
206 | QObject::connect(m_ioworker, &DeleteJobIOWorker::rmddirResult, q, [=](bool result, const QUrl &url){ | ||||
207 | this->rmdirResult(result, url); | ||||
208 | }); | ||||
209 | m_thread->start(); | ||||
210 | } | ||||
211 | | ||||
212 | return m_ioworker; | ||||
213 | } | ||||
214 | | ||||
151 | void DeleteJobPrivate::slotReport() | 215 | void DeleteJobPrivate::slotReport() | ||
152 | { | 216 | { | ||
153 | Q_Q(DeleteJob); | 217 | Q_Q(DeleteJob); | ||
154 | emit q->deleting(q, m_currentURL); | 218 | emit q->deleting(q, m_currentURL); | ||
155 | 219 | | |||
156 | // TODO: maybe we could skip everything else when (flags & HideProgressInfo) ? | 220 | // TODO: maybe we could skip everything else when (flags & HideProgressInfo) ? | ||
157 | JobPrivate::emitDeleting(q, m_currentURL); | 221 | JobPrivate::emitDeleting(q, m_currentURL); | ||
158 | 222 | | |||
▲ Show 20 Lines • Show All 115 Lines • ▼ Show 20 Line(s) | 331 | { | |||
274 | const QSet<QString>::const_iterator itEnd = m_parentDirs.constEnd(); | 338 | const QSet<QString>::const_iterator itEnd = m_parentDirs.constEnd(); | ||
275 | for (QSet<QString>::const_iterator it = m_parentDirs.constBegin(); it != itEnd; ++it) { | 339 | for (QSet<QString>::const_iterator it = m_parentDirs.constBegin(); it != itEnd; ++it) { | ||
276 | KDirWatch::self()->stopDirScan(*it); | 340 | KDirWatch::self()->stopDirScan(*it); | ||
277 | } | 341 | } | ||
278 | state = DELETEJOB_STATE_DELETING_FILES; | 342 | state = DELETEJOB_STATE_DELETING_FILES; | ||
279 | deleteNextFile(); | 343 | deleteNextFile(); | ||
280 | } | 344 | } | ||
281 | 345 | | |||
282 | void DeleteJobPrivate::deleteNextFile() | 346 | | ||
347 | void DeleteJobPrivate::rmFileResult(bool result, const QUrl &url, bool isLink) | ||||
283 | { | 348 | { | ||
284 | Q_Q(DeleteJob); | 349 | if (result) { | ||
285 | //qDebug(); | | |||
286 | if (!files.isEmpty() || !symlinks.isEmpty()) { | | |||
287 | SimpleJob *job; | | |||
288 | do { | | |||
289 | // Take first file to delete out of list | | |||
290 | QList<QUrl>::iterator it = files.begin(); | | |||
291 | bool isLink = false; | | |||
292 | if (it == files.end()) { // No more files | | |||
293 | it = symlinks.begin(); // Pick up a symlink to delete | | |||
294 | isLink = true; | | |||
295 | } | | |||
296 | // Normal deletion | | |||
297 | // If local file, try do it directly | | |||
298 | if ((*it).isLocalFile() && QFile::remove((*it).toLocalFile())) { | | |||
299 | //kdDebug(7007) << "DeleteJob deleted" << (*it).toLocalFile(); | | |||
300 | job = nullptr; | | |||
301 | m_processedFiles++; | 350 | m_processedFiles++; | ||
302 | if (m_processedFiles % 300 == 1 || m_totalFilesDirs < 300) { // update progress info every 300 files | 351 | | ||
303 | m_currentURL = *it; | 352 | if (isLink) { | ||
dfaure: That is a really weird comment, for a bool called isLink :-) | |||||
304 | slotReport(); | 353 | symlinks.removeFirst(); | ||
354 | } else { | ||||
355 | files.removeFirst(); | ||||
305 | } | 356 | } | ||
357 | | ||||
358 | deleteNextFile(); | ||||
306 | } else { | 359 | } else { | ||
307 | // if remote - or if unlink() failed (we'll use the job's error handling in that case) | 360 | // fallback if QFile::remove() failed (we'll use the job's error handling in that case) | ||
dfaure: (pre-existing) s/unlink/QFile::remove/ in this comment would be less confusing | |||||
308 | //qDebug() << "calling file_delete on" << *it; | 361 | deleteFileUsingJob(url, isLink); | ||
309 | if (isHttpProtocol(it->scheme())) { | 362 | } | ||
310 | job = KIO::http_delete(*it, KIO::HideProgressInfo); | 363 | } | ||
364 | | ||||
365 | void DeleteJobPrivate::deleteFileUsingJob(const QUrl &url, bool isLink) | ||||
dfaure: please remove spaces after method names everywhere | |||||
366 | { | ||||
367 | Q_Q(DeleteJob); | ||||
(With the change I suggest below, this would just move to after m_processedFiles++; above. This would simplify the control flow in this method) dfaure: (With the change I suggest below, this would just move to after `m_processedFiles++;` above. | |||||
368 | | ||||
369 | SimpleJob *job; | ||||
370 | if (isHttpProtocol(url.scheme())) { | ||||
371 | job = KIO::http_delete(url, KIO::HideProgressInfo); | ||||
No space after the method name. This method could take a QUrl instead of an iterator, all it does is *it to get to the QUrl. Even better, it could keep using an iterator, but also take care of of the two erase(it) calls and addSubjob. This is currently duplicated between the two callers. The only difference is bool isLink, you could pass that as parameter. This would even be more consistent with DeleteJobPrivate::deleteDirUsingJob which takes care of erase(it) and addSubjob. dfaure: No space after the method name.
This method could take a QUrl instead of an iterator, all it… | |||||
311 | } else { | 372 | } else { | ||
312 | job = KIO::file_delete(*it, KIO::HideProgressInfo); | 373 | job = KIO::file_delete(url, KIO::HideProgressInfo); | ||
313 | job->setParentJob(q); | 374 | job->setParentJob(q); | ||
314 | } | 375 | } | ||
315 | Scheduler::setJobPriority(job, 1); | 376 | Scheduler::setJobPriority(job, 1); | ||
316 | m_currentURL = (*it); | 377 | | ||
317 | } | | |||
318 | if (isLink) { | 378 | if (isLink) { | ||
319 | symlinks.erase(it); | 379 | symlinks.removeFirst(); | ||
320 | } else { | 380 | } else { | ||
321 | files.erase(it); | 381 | files.removeFirst(); | ||
322 | } | 382 | } | ||
323 | if (job) { | 383 | | ||
324 | q->addSubjob(job); | 384 | q->addSubjob(job); | ||
325 | return; | | |||
326 | } | 385 | } | ||
327 | // loop only if direct deletion worked (job=0) and there is something else to delete | 386 | | ||
328 | } while (!job && (!files.isEmpty() || !symlinks.isEmpty())); | 387 | void DeleteJobPrivate::deleteNextFile() | ||
388 | { | ||||
389 | //qDebug(); | ||||
390 | | ||||
391 | // if there is something else to delete | ||||
392 | // the loop is run using callbacks slotResult and rmFileResult | ||||
393 | if (!files.isEmpty() || !symlinks.isEmpty()) { | ||||
394 | | ||||
395 | // Take first file to delete out of list | ||||
dfaure: You can make it `const` then. | |||||
396 | QList<QUrl>::iterator it = files.begin(); | ||||
397 | const bool isLink = (it == files.end()); // No more files | ||||
398 | if (isLink) { | ||||
399 | it = symlinks.begin(); // Pick up a symlink to delete | ||||
329 | } | 400 | } | ||
401 | m_currentURL = (*it); | ||||
402 | | ||||
403 | // If local file, try do it directly | ||||
404 | if (m_currentURL.isLocalFile()){ | ||||
dfaure: I suggest to remove the erroneous comment | |||||
dfaure: space before `{` here as well | |||||
405 | // separate thread will do the work | ||||
406 | QMetaObject::invokeMethod(worker(), "rmfile", Qt::QueuedConnection, | ||||
407 | Q_ARG(const QUrl&, m_currentURL), | ||||
dfaure: Since you're already using m_currentURL here, I'd say this is acceptable. | |||||
408 | Q_ARG(bool, isLink)); | ||||
409 | } else { | ||||
410 | // if remote, use a job | ||||
411 | deleteFileUsingJob(m_currentURL, isLink); | ||||
412 | } | ||||
413 | return; | ||||
414 | } | ||||
415 | | ||||
330 | state = DELETEJOB_STATE_DELETING_DIRS; | 416 | state = DELETEJOB_STATE_DELETING_DIRS; | ||
331 | deleteNextDir(); | 417 | deleteNextDir(); | ||
332 | } | 418 | } | ||
333 | 419 | | |||
334 | void DeleteJobPrivate::deleteNextDir() | 420 | void DeleteJobPrivate::rmdirResult(bool result, const QUrl &url) | ||
335 | { | 421 | { | ||
336 | Q_Q(DeleteJob); | 422 | if (result) { | ||
337 | if (!dirs.isEmpty()) { // some dirs to delete ? | | |||
338 | do { | | |||
339 | // Take first dir to delete out of list - last ones first ! | | |||
340 | QList<QUrl>::iterator it = --dirs.end(); | | |||
341 | // If local dir, try to rmdir it directly | | |||
342 | if ((*it).isLocalFile() && QDir().rmdir((*it).toLocalFile())) { | | |||
343 | m_processedDirs++; | 423 | m_processedDirs++; | ||
344 | if (m_processedDirs % 100 == 1) { // update progress info every 100 dirs | 424 | dirs.removeLast(); | ||
dfaure: removeLast here too? | |||||
dfaure: marked as done but I still see removeFirst, I'm confused. | |||||
meven: I missed this line, I did it line 430. | |||||
345 | m_currentURL = *it; | 425 | deleteNextDir(); | ||
346 | slotReport(); | | |||
347 | } | | |||
348 | } else { | 426 | } else { | ||
427 | // fallback | ||||
428 | deleteDirUsingJob(url); | ||||
429 | } | ||||
430 | } | ||||
431 | | ||||
432 | void DeleteJobPrivate::deleteDirUsingJob (const QUrl &url) | ||||
dfaure: I still see a space between method name and `(` here :-) | |||||
433 | { | ||||
434 | Q_Q(DeleteJob); | ||||
435 | | ||||
349 | // Call rmdir - works for kioslaves with canDeleteRecursive too, | 436 | // Call rmdir - works for kioslaves with canDeleteRecursive too, | ||
350 | // CMD_DEL will trigger the recursive deletion in the slave. | 437 | // CMD_DEL will trigger the recursive deletion in the slave. | ||
351 | SimpleJob *job = KIO::rmdir(*it); | 438 | SimpleJob *job = KIO::rmdir(url); | ||
352 | job->setParentJob(q); | 439 | job->setParentJob(q); | ||
353 | job->addMetaData(QStringLiteral("recurse"), QStringLiteral("true")); | 440 | job->addMetaData(QStringLiteral("recurse"), QStringLiteral("true")); | ||
354 | Scheduler::setJobPriority(job, 1); | 441 | Scheduler::setJobPriority(job, 1); | ||
355 | dirs.erase(it); | 442 | dirs.removeLast(); | ||
356 | q->addSubjob(job); | 443 | q->addSubjob(job); | ||
357 | return; | | |||
358 | } | 444 | } | ||
359 | dirs.erase(it); | 445 | | ||
360 | } while (!dirs.isEmpty()); | 446 | void DeleteJobPrivate::deleteNextDir() | ||
447 | { | ||||
448 | Q_Q(DeleteJob); | ||||
449 | | ||||
450 | if (!dirs.isEmpty()) { // some dirs to delete ? | ||||
451 | | ||||
452 | // the loop is run using callbacks slotResult and rmdirResult | ||||
453 | // Take first dir to delete out of list - last ones first ! | ||||
454 | QList<QUrl>::iterator it = --dirs.end(); | ||||
455 | m_currentURL = (*it); | ||||
456 | // If local dir, try to rmdir it directly | ||||
457 | if (m_currentURL.isLocalFile()) { | ||||
Use m_currentURL here, or use (*it) in the Q_ARG. It's just inconsistent right now, even though of course it's all technically the same. (I think the code wasn't using m_currentURL because that's just something used for reporting, added after the fact, and which could technically be changed again one day -- although that's unlikely, I guess). dfaure: Use m_currentURL here, or use (*it) in the Q_ARG. It's just inconsistent right now, even though… | |||||
458 | // delete it on separate worker thread | ||||
459 | QMetaObject::invokeMethod(worker(), "rmdir", Qt::QueuedConnection, | ||||
460 | Q_ARG(const QUrl&, m_currentURL)); | ||||
461 | } else { | ||||
dfaure: Move both `return` to after the `if/else`? | |||||
462 | deleteDirUsingJob(m_currentURL); | ||||
463 | } | ||||
464 | return; | ||||
361 | } | 465 | } | ||
362 | 466 | | |||
363 | // Re-enable watching on the dirs that held the deleted files | 467 | // Re-enable watching on the dirs that held the deleted files | ||
364 | restoreDirWatch(); | 468 | restoreDirWatch(); | ||
365 | 469 | | |||
366 | // Finished - tell the world | 470 | // Finished - tell the world | ||
367 | if (!m_srcList.isEmpty()) { | 471 | if (!m_srcList.isEmpty()) { | ||
368 | //qDebug() << "KDirNotify'ing FilesRemoved" << m_srcList; | 472 | //qDebug() << "KDirNotify'ing FilesRemoved" << m_srcList; | ||
▲ Show 20 Lines • Show All 137 Lines • ▼ Show 20 Line(s) | |||||
506 | { | 610 | { | ||
507 | DeleteJob *job = DeleteJobPrivate::newJob(src, flags); | 611 | DeleteJob *job = DeleteJobPrivate::newJob(src, flags); | ||
508 | if (job->uiDelegateExtension()) { | 612 | if (job->uiDelegateExtension()) { | ||
509 | job->uiDelegateExtension()->createClipboardUpdater(job, JobUiDelegateExtension::RemoveContent); | 613 | job->uiDelegateExtension()->createClipboardUpdater(job, JobUiDelegateExtension::RemoveContent); | ||
510 | } | 614 | } | ||
511 | return job; | 615 | return job; | ||
512 | } | 616 | } | ||
513 | 617 | | |||
618 | #include "deletejob.moc" | ||||
514 | #include "moc_deletejob.cpp" | 619 | #include "moc_deletejob.cpp" |
can be removed