D21760: Add KListOpenFiles::listProcessesWithOpenFiles

[David Faure]

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


  The KJob usage turned out exactly like I thought it would, I still think it's the right thing to do :-)

INLINE COMMENTS

> hallas wrote in klistopenfiles.cpp:29
> The primary motivation was memory-management, but I could also just make the d-pointer in ListOpenFilesJob a QScopedPointer, would that be better?

Yes, that would be more lightweight.

> hallas wrote in klistopenfiles.cpp:44
> I have added the error enum entries, but I am a little hesitant to add to error texts since KCoreAddons doesn't today depend on KDE::I18n and this would introduce that dependency, do we really want that? When I look at the KIO classes then they usually set som non-translated string as the error text, would that be an option?

My bad. In KCoreAddons should should use tr()+arg() rather than i18n.

KIO doesn't have non-translated strings. It reimplements errorString() to combine the errorText (usually a path or URL) with a translated string based on the error code.
But the base KJob doesn't have any of that, it just defaults to errorText, so you *need* to use tr().

REPOSITORY
  R244 KCoreAddons

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

To: hallas, davidedmundson, broulik, #frameworks, dfaure, bruns, #plasma
Cc: meven, cfeck, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20190830/06fac30e/attachment.html>