D11365: also test for value types in taglibextractortest and fix errors

Michael Heidelbach noreply at phabricator.kde.org
Thu Mar 29 09:40:04 UTC 2018


michaelh requested changes to this revision.
michaelh added a comment.


  This patch should be split.
  
  1. Test more properties
  2. Change return types of ...
  
  Also 'fix errors' in the title is misleading because currently kfilemetadata works well.

INLINE COMMENTS

> astippich wrote in taglibextractor.cpp:363
> That was actually the point for this. In propertyinfo, it says that this property is a stringlist, but it was actually never one, just a simple string. 
> For multiple values, there will be multiple entries in the property map with a single string.  This is not considered within Elisa at least. We could also go the other way and adjust the properties to match the behavior. 
> @michaelh agreed that stringlists are easier to handle

You have to ensure baloo searching still works, when string lists contains more than 1 items. I think it will break.

Generally IMO returning a string list is the natural thing to do here. 
On the other hand: Why is `PropertyInfo` announcing the value type when calling `.type()` on a QVariant would be sufficient. There must be a reason for using `PropertyMap` and giving a type hint in `PropertyInfo`. Unless somebody knows the reason this will need some investigation and a concerted action if we choose to change it.
I've put D10694 <https://phabricator.kde.org/D10694> on hold because of that.

> astippich wrote in taglibextractor.cpp:393
> This one is a litte bit more tricky, as the year property could theoretically also be negative (B.C.). Not really relevant for music, but maybe for written documents, and still only very rarily :)

It depends on how you define 'releaseYear' if you think of it as year published negative dates can be ruled out. ;-)

REPOSITORY
  R286 KFileMetaData

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

To: astippich, #frameworks, #baloo, mgallien, michaelh
Cc: michaelh, #frameworks, ashaposhnikov, astippich, spoorun, nicolasfella, ngraham, alexeymin
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20180329/902bf25f/attachment-0001.html>


More information about the Kde-frameworks-devel mailing list