Changeset View
Standalone View
src/panels/places/placesitemmodel.cpp
Show All 17 Lines | |||||
18 | * You should have received a copy of the GNU General Public License * | 18 | * You should have received a copy of the GNU General Public License * | ||
19 | * along with this program; if not, write to the * | 19 | * along with this program; if not, write to the * | ||
20 | * Free Software Foundation, Inc., * | 20 | * Free Software Foundation, Inc., * | ||
21 | * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA * | 21 | * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA * | ||
22 | ***************************************************************************/ | 22 | ***************************************************************************/ | ||
23 | 23 | | |||
24 | #include "placesitemmodel.h" | 24 | #include "placesitemmodel.h" | ||
25 | 25 | | |||
26 | #include "config-ksysguard.h" | ||||
26 | #include "dolphin_generalsettings.h" | 27 | #include "dolphin_generalsettings.h" | ||
27 | #include "dolphindebug.h" | 28 | #include "dolphindebug.h" | ||
28 | #include "dolphinplacesmodelsingleton.h" | 29 | #include "dolphinplacesmodelsingleton.h" | ||
29 | #include "placesitem.h" | 30 | #include "placesitem.h" | ||
30 | #include "placesitemsignalhandler.h" | 31 | #include "placesitemsignalhandler.h" | ||
31 | #include "views/dolphinview.h" | 32 | #include "views/dolphinview.h" | ||
32 | #include "views/viewproperties.h" | 33 | #include "views/viewproperties.h" | ||
33 | 34 | | |||
34 | #include <KAboutData> | 35 | #include <KAboutData> | ||
35 | #include <KLocalizedString> | 36 | #include <KLocalizedString> | ||
36 | #include <KUrlMimeData> | 37 | #include <KUrlMimeData> | ||
37 | #include <Solid/DeviceNotifier> | 38 | #include <Solid/DeviceNotifier> | ||
38 | #include <Solid/OpticalDrive> | 39 | #include <Solid/OpticalDrive> | ||
39 | 40 | | |||
41 | #ifdef HAVE_KSYSGUARD | ||||
42 | #include <ksysguard/processcore/process.h> | ||||
43 | #include <ksysguard/processcore/processes.h> | ||||
44 | #endif | ||||
45 | | ||||
40 | #include <QAction> | 46 | #include <QAction> | ||
41 | #include <QIcon> | 47 | #include <QIcon> | ||
42 | #include <QMimeData> | 48 | #include <QMimeData> | ||
43 | #include <QTimer> | 49 | #include <QTimer> | ||
44 | 50 | | |||
51 | #ifdef HAVE_KSYSGUARD | ||||
52 | #include <QProcess> | ||||
53 | #endif | ||||
54 | | ||||
45 | namespace { | 55 | namespace { | ||
46 | static QList<QUrl> balooURLs = { | 56 | static QList<QUrl> balooURLs = { | ||
47 | QUrl(QStringLiteral("timeline:/today")), | 57 | QUrl(QStringLiteral("timeline:/today")), | ||
48 | QUrl(QStringLiteral("timeline:/yesterday")), | 58 | QUrl(QStringLiteral("timeline:/yesterday")), | ||
49 | QUrl(QStringLiteral("timeline:/thismonth")), | 59 | QUrl(QStringLiteral("timeline:/thismonth")), | ||
50 | QUrl(QStringLiteral("timeline:/lastmonth")), | 60 | QUrl(QStringLiteral("timeline:/lastmonth")), | ||
51 | QUrl(QStringLiteral("search:/documents")), | 61 | QUrl(QStringLiteral("search:/documents")), | ||
52 | QUrl(QStringLiteral("search:/images")), | 62 | QUrl(QStringLiteral("search:/images")), | ||
▲ Show 20 Lines • Show All 413 Lines • ▼ Show 20 Line(s) | |||||
466 | 476 | | |||
467 | void PlacesItemModel::updateItem(PlacesItem *item, const QModelIndex &index) | 477 | void PlacesItemModel::updateItem(PlacesItem *item, const QModelIndex &index) | ||
468 | { | 478 | { | ||
469 | item->setGroup(index.data(KFilePlacesModel::GroupRole).toString()); | 479 | item->setGroup(index.data(KFilePlacesModel::GroupRole).toString()); | ||
470 | item->setIcon(index.data(KFilePlacesModel::IconNameRole).toString()); | 480 | item->setIcon(index.data(KFilePlacesModel::IconNameRole).toString()); | ||
471 | item->setGroupHidden(index.data(KFilePlacesModel::GroupHiddenRole).toBool()); | 481 | item->setGroupHidden(index.data(KFilePlacesModel::GroupHiddenRole).toBool()); | ||
472 | } | 482 | } | ||
473 | 483 | | |||
484 | void PlacesItemModel::queryBlockingApps(const QString& devicePath) | ||||
hallas: I just copied this code from the Device Notifier, but it would be nice to have this as general… | |||||
485 | { | ||||
486 | #ifdef HAVE_KSYSGUARD | ||||
487 | QProcess *p = new QProcess; | ||||
488 | connect(p, static_cast<void (QProcess::*)(QProcess::ProcessError)>(&QProcess::error), [=](QProcess::ProcessError) { | ||||
489 | emit blockingAppsReady({}); | ||||
490 | p->deleteLater(); | ||||
491 | }); | ||||
492 | connect(p, static_cast<void (QProcess::*)(int,QProcess::ExitStatus)>(&QProcess::finished), [=](int,QProcess::ExitStatus) { | ||||
493 | QStringList blockApps; | ||||
494 | QString out(p->readAll()); | ||||
495 | const QStringList &pidList = out.split(QRegExp(QStringLiteral("\\s+")), QString::SkipEmptyParts); | ||||
496 | KSysGuard::Processes procs; | ||||
497 | Q_FOREACH (const QString &pidStr, pidList) { | ||||
498 | int pid = pidStr.toInt(); | ||||
499 | if (!pid) { | ||||
500 | continue; | ||||
501 | } | ||||
502 | procs.updateOrAddProcess(pid); | ||||
503 | KSysGuard::Process *proc = procs.getProcess(pid); | ||||
504 | if (!blockApps.contains(proc->name())) { | ||||
505 | blockApps << proc->name(); | ||||
506 | } | ||||
507 | } | ||||
508 | blockApps.removeDuplicates(); | ||||
509 | emit blockingAppsReady(blockApps); | ||||
510 | p->deleteLater(); | ||||
511 | }); | ||||
512 | p->start(QStringLiteral("lsof"), {QStringLiteral("-t"), devicePath}); | ||||
513 | #endif | ||||
514 | } | ||||
515 | | ||||
474 | void PlacesItemModel::slotStorageTearDownDone(Solid::ErrorType error, const QVariant& errorData) | 516 | void PlacesItemModel::slotStorageTearDownDone(Solid::ErrorType error, const QVariant& errorData) | ||
475 | { | 517 | { | ||
476 | if (error && errorData.isValid()) { | 518 | if (error && errorData.isValid()) { | ||
519 | #ifdef HAVE_KSYSGUARD | ||||
520 | if (error == Solid::ErrorType::DeviceBusy) { | ||||
521 | // Without that, our lambda function would capture an uninitialized object, resulting in UB | ||||
522 | // and random crashes | ||||
523 | QMetaObject::Connection *c = new QMetaObject::Connection(); | ||||
524 | *c = connect(this, &PlacesItemModel::blockingAppsReady, [=] (const QStringList &blockApps) { | ||||
525 | QString errorString; | ||||
526 | if (blockApps.isEmpty()) { | ||||
527 | errorString = i18n("One or more files on this device are open within an application."); | ||||
528 | } else { | ||||
529 | errorString = i18np("One or more files on this device are opened in application \"%2\".", | ||||
What do you think about the wording of these error messages? Currently I just copied the ones from the Device Notifier, but actually I think that it should also state that unmounting has filed, or maybe that Removal has failed. hallas: What do you think about the wording of these error messages? Currently I just copied the ones… | |||||
elvisangelaccio: Feel free to add those errors ;) | |||||
530 | "One or more files on this device are opened in following applications: %2.", | ||||
531 | blockApps.count(), blockApps.join(i18nc("separator in list of apps blocking device unmount", ", "))); | ||||
532 | } | ||||
533 | emit errorMessage(errorString); | ||||
534 | disconnect(*c); | ||||
535 | delete c; | ||||
536 | }); | ||||
537 | queryBlockingApps(m_deviceToTearDown->filePath()); | ||||
538 | return; | ||||
539 | } | ||||
540 | #endif | ||||
477 | emit errorMessage(errorData.toString()); | 541 | emit errorMessage(errorData.toString()); | ||
478 | } | 542 | } | ||
479 | disconnect(m_deviceToTearDown, &Solid::StorageAccess::teardownDone, | 543 | disconnect(m_deviceToTearDown, &Solid::StorageAccess::teardownDone, | ||
elvisangelaccio: Missing `this` as receiver. | |||||
I meant connect(listOpenFilesJob, &KIO::Job::result, this, [...](...) { ... }); Without a receiver, the connection could lead to random crashes. elvisangelaccio: I meant
```
connect(listOpenFilesJob, &KIO::Job::result, this, [...](...) { ... });
```… | |||||
I am not sure I understand what you mean? So is what you are saying that I need to connect the signal to, e.g. a member function? hallas: I am not sure I understand what you mean? So is what you are saying that I need to connect the… | |||||
I believe he means : connect(listOpenFilesJob, &KIO::Job::result, **this**, [this, listOpenFilesJob](KJob*) {... meven: I believe he means : `connect(listOpenFilesJob, &KIO::Job::result, **this**, [this… | |||||
hallas: I have made this change, hope it is what you meant :) | |||||
Yes. Without a receiver, a lambda slot could lead to crashes. More details here: https://medium.com/genymobile/how-c-lambda-expressions-can-improve-your-qt-code-8cd524f4ed9f elvisangelaccio: Yes. Without a receiver, a lambda slot could lead to crashes. More details here: https://medium. | |||||
480 | this, &PlacesItemModel::slotStorageTearDownDone); | 544 | this, &PlacesItemModel::slotStorageTearDownDone); | ||
elvisangelaccio: Missing `const` | |||||
481 | m_deviceToTearDown = nullptr; | 545 | m_deviceToTearDown = nullptr; | ||
482 | } | 546 | } | ||
483 | 547 | | |||
484 | void PlacesItemModel::slotStorageSetupDone(Solid::ErrorType error, | 548 | void PlacesItemModel::slotStorageSetupDone(Solid::ErrorType error, | ||
485 | const QVariant& errorData, | 549 | const QVariant& errorData, | ||
486 | const QString& udi) | 550 | const QString& udi) | ||
487 | { | 551 | { | ||
488 | Q_UNUSED(udi); | 552 | Q_UNUSED(udi); | ||
489 | 553 | | |||
490 | const int index = m_storageSetupInProgress.take(sender()); | 554 | const int index = m_storageSetupInProgress.take(sender()); | ||
What do you think about the wording of these error messages? Currently I just copied the ones from the Device Notifier, but actually I think that it should also state that unmounting has filed, or maybe that Removal has failed. hallas: What do you think about the wording of these error messages? Currently I just copied the ones… | |||||
Please use the <application> tag. More details in: https://api.kde.org/frameworks/ki18n/html/prg_guide.html#kuit_markup elvisangelaccio: Please use the `<application>` tag. More details in: https://api.kde. | |||||
491 | const PlacesItem* item = placesItem(index); | 555 | const PlacesItem* item = placesItem(index); | ||
elvisangelaccio: Same here | |||||
492 | if (!item) { | 556 | if (!item) { | ||
493 | return; | 557 | return; | ||
494 | } | 558 | } | ||
495 | 559 | | |||
496 | if (error != Solid::NoError) { | 560 | if (error != Solid::NoError) { | ||
497 | if (errorData.isValid()) { | 561 | if (errorData.isValid()) { | ||
498 | emit errorMessage(i18nc("@info", "An error occurred while accessing '%1', the system responded: %2", | 562 | emit errorMessage(i18nc("@info", "An error occurred while accessing '%1', the system responded: %2", | ||
499 | item->text(), | 563 | item->text(), | ||
▲ Show 20 Lines • Show All 277 Lines • Show Last 20 Lines |
I just copied this code from the Device Notifier, but it would be nice to have this as general functionality somewhere