D19989: Unmounting busy device doesn't tell who is blocking

[Elvis Angelaccio]

elvisangelaccio requested changes to this revision.
elvisangelaccio added a comment.
This revision now requires changes to proceed.


  Please update the commit message, it is no longer true that "this commit copies the code from there to achieve the same thing."

INLINE COMMENTS

> hallas wrote in CMakeLists.txt:11
> `KListOpenFilesJob` is available from KDE Frameworks 5.63.0 so I updated this min version, is there a process around this so that packagers are notified about this?

It's ok to bump the KF5 version as long as we do it before the dependency freeze. No need to notify the packagers.

> placesitemmodel.cpp:481
> +            KListOpenFilesJob* listOpenFilesJob = new KListOpenFilesJob(m_deviceToTearDown->filePath());
> +            connect(listOpenFilesJob, &KIO::Job::result, [=](KJob*) {
> +                KProcessList::KProcessInfoList blockingProcesses = listOpenFilesJob->processInfoList();

Missing `this` as receiver.

> placesitemmodel.cpp:492
> +                    blockingApps.removeDuplicates();
> +                    errorString = i18np("One or more files on this device are opened in application \"%2\".",
> +                            "One or more files on this device are opened in following applications: %2.",

Please use the `<application>` tag. More details in: https://api.kde.org/frameworks/ki18n/html/prg_guide.html#kuit_markup

> placesitemmodel.cpp:493
> +                    errorString = i18np("One or more files on this device are opened in application \"%2\".",
> +                            "One or more files on this device are opened in following applications: %2.",
> +                            blockingApps.count(), blockingApps.join(i18nc("separator in list of apps blocking device unmount", ", ")));

Same here

> hallas wrote in placesitemmodel.cpp:529
> 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.

Feel free to add those errors ;)

REPOSITORY
  R318 Dolphin

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

To: hallas, #dolphin, elvisangelaccio, ngraham, broulik, meven
Cc: meven, davidedmundson, kfm-devel, iasensio, fprice, MrPepe, fbampaloukas, alexde, Codezela, feverfew, spoorun, navarromorales, firef, ngraham, andrebarros, emmanuelp, mikesomov
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20191020/df6f081e/attachment.htm>