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

[Alexander Stippich]

astippich added inline comments.

INLINE COMMENTS

> mgallien wrote in taglibextractor.cpp:363
> Is it really needed to push directly a list ?
> You are ignoring the fact that the original developer intended add to be called once for each value. I would prefer to keep doing that until we can be absolutely sure that nothing breaks if we change that.

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

> mgallien wrote in taglibextractor.cpp:389
> Are you sure we need this cast ? I do not think we will ever need to have negative track numbers added. The original type is unsigned int and should exactly convey the fact that we do not expect negative numbers.

I changed it so it matches its associated value type, I could also change the type in propertyinfo to uint

> mgallien wrote in taglibextractor.cpp:393
> idem

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 :)

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/20180328/37426215/attachment.html>