D10694: epubextractor: Handle multiple subjects better

[Matthieu Gallien]

mgallien added a comment.


  Sorry for being so late.
  
  In D10694#215663 <https://phabricator.kde.org/D10694#215663>, @michaelh wrote:
  
  > @mgallien : taglibextractor.cpp is very hard to read. Hopefully it is no longer.

INLINE COMMENTS

> michaelh wrote in epubextractor.cpp:94
> Did you mean something like this?:
> 
>   const auto& values = fetchMetadata(ePubDoc, EPUB_SUBJECT);
>   for (auto& value : values) {
>           result->add(Property::Subject, value);
>    }
> 
> It fails the test (of course). It it unclear to me how to handle `result.properties().value(Property::Subject)` and there is no example in the tests. They all compare against `QVariant::Type::String`. Do you have an example from elisa?

Elisa code is not properly handling this case. When writing it, I had not understood that. Thanks to your diff, I have now properly read the code and I should go modify Elisa code.

In general, you can use canConvert<the type>() from QVariant. This should allow to handle both cases.
You can probably always convert to a string list QVariant::toStringList. It should be enough for properties with one string or a list of strings.

REPOSITORY
  R286 KFileMetaData

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

To: michaelh, mgallien, dfaure
Cc: #frameworks, ashaposhnikov, michaelh, spoorun, navarromorales, isidorov, nicolasfella, firef, andrebarros, alexeymin, emmanuelp
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20180308/d809f47e/attachment.html>