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
