astippich added a comment. Restricted Application added a project: Baloo.
In D10803#215270 <https://phabricator.kde.org/D10803#215270>, @mgallien wrote: > In D10803#214655 <https://phabricator.kde.org/D10803#214655>, @astippich wrote: > > > In D10803#213783 <https://phabricator.kde.org/D10803#213783>, @michaelh wrote: > > > > > In D10803#213767 <https://phabricator.kde.org/D10803#213767>, @astippich wrote: > > > > > > > I don't know dolphin works, but given how KFileMetadata is designed, the new tags should be ignored until explicit support is added in dolphin. > > > > > > > > > Please try that. For unknown types baloo-widgets (responsible for infopanel/tooltips) falls back to `value.toString()`, no idea what will happen when you feed it with binary data. The cover properties return binary data, right? > > > > > > The cover is binary data, right. And I think I was wrong. While I don't fully understand yet how baloo-widgets work, from what I've read all available properties are automatically queried. The binary data would then be converted to a string as you said. So adding the cover this way may not be a good solution. A different interface should be created for image/binary data. > > I think I will split this into two diffs and do the cover images separately. > > > This is not necessary. You could use a QImage or a QPixmap. You can put them in a QVariant and they should be converted to an empty QString. > As far as I understand, in Elisa, we would need to access them with QQuickImageProvider. All in all, well done job. > > I am not sure for the rating given that there is also a rating (the one used in Elisa) that is stored as an extended file system attribute independent from the file mime type. One limit of this rating is that it is specific to the KFileMetaData API users. Do you have an idea ? I think QImage is no different than QByteArray for baloo-widgets. With QByteArray, it shows a garbage string. With QImage, it still displays the label and an empty string afterwards. So not really a nice solution as can be seen in the picture. One could add a special method to ExtactionResult to query the picture data, but I think this breaks ABI. I think the issue here is baloo-widgets unconditionally displaying everything it gets while it should actually check for the type. Judging from a first glance, this should be possible, but since baloo widgets is in kde applications and has a different release schedule, one could still see the issue when a new kfilemetadata is used with an older kde applications release. Is this acceptable? F5733004: Screenshot_20180227_233349.png <https://phabricator.kde.org/F5733004> Regarding the rating, I'm also not entirely happy about this. I implemented it because I thought it would be a nice fallback for Elisa if the file has not been rated before (and maybe also for different operating systems). We also had this as a feature request somewhere. But there is no real standard regarding how the rating is stored and it is probably hard to get it right. So right know I'm inclined to remove that tag again. Thoughts? Btw, lots of more tags incoming In D10803#215305 <https://phabricator.kde.org/D10803#215305>, @michaelh wrote: > In D10803#215270 <https://phabricator.kde.org/D10803#215270>, @mgallien wrote: > > > This is not necessary. You could use a QImage or a QPixmap. You can put them in a QVariant and they should be converted to an empty QString. > > > Please consider writing a plugin for KIO:PreviewJob (for the covers) anyway. It would be more widely useable. I'd like to see a/the cover/s in Dolphin too. That would be a nice feature indeed. REPOSITORY R286 KFileMetaData REVISION DETAIL https://phabricator.kde.org/D10803 To: astippich, mgallien Cc: dfaure, michaelh, ngraham, #frameworks, ashaposhnikov, spoorun, nicolasfella, alexeymin
