marcingu added a comment.
In D28745#676317 <https://phabricator.kde.org/D28745#676317>, @bruns wrote: > In D28745#676313 <https://phabricator.kde.org/D28745#676313>, @marcingu wrote: > > > Ping! > > I'm remanding about question early, because I could do much more work if I get to do it on weekend. > > > > Question: > > This code won't save thumbnail for file on any device that isn't `StorageVolume` or is `StorageVolume` with `usage` `UsageType::Encrypded`. > > > The whole block can never return true, so it should just be removed, along with all its dependencies. I tested it once more and it returns true when it should, as expected. What makes you think it doesn't? >> Is this fine? >> Should we take something else into consideration? >> Do we want that feature tested to avoid regression in the future? > > This code duplicates functionality already present in the thumbnailer code in KIO core. It can be replaced by a trivial "CacheThumbnail" flag provided by the caller. I wasn't able to prevent catching of directory thumbnails from KIO. The fact that files used for making a preview can be on different storage, makes it extra tricky. But I'm new the project, so if you do know how to make it simpler I'm all ears. REPOSITORY R320 KIO Extras REVISION DETAIL https://phabricator.kde.org/D28745 To: marcingu, ivan, broulik, #dolphin, ngraham, meven, bruns, dfaure Cc: dfaure, thiago, bruns, meven, ngraham, kde-frameworks-devel, kfm-devel, waitquietly, azyx, nikolaik, pberestov, iasensio, aprcela, fprice, LeGast00n, cblack, fbampaloukas, alexde, Codezela, feverfew, michaelh, spoorun, navarromorales, firef, andrebarros, emmanuelp, rdieter, mikesomov