D10694: epubextractor: Handle multiple subjects better

[Matthieu Gallien]

mgallien requested changes to this revision.
mgallien added a comment.
This revision now requires changes to proceed.


  In D10694#214996 <https://phabricator.kde.org/D10694#214996>, @michaelh wrote:
  
  > In D10694#213712 <https://phabricator.kde.org/D10694#213712>, @mgallien wrote:
  >
  > > To me, it is necessary to have a test that ensures that possible existing clients are not affected by your change. Could you add it ?
  >
  >
  > The new behaviour will break clients. No test needed for this. E.g. baloo-widgets will show the label 'Subject' but no content.
  >  To make it clear: I do not want break this for clients apart from baloo-widgets. The question is: Who are the clients? and How to find them except for searching https://lxr.kde.org/? Please help me with this.
  
  
  Yesterday, I tried looking for such use and did not found them. I did try to use web search engine with no success. I am afraid I cannot help you but would be happy to hear any advice.
  
  > The current behaviour is already breaking: I have some epub files with a lot of `dc:subject` tags. The semicolon-concatenated string is longer than my monitor is wide. As a result in dolphin the tooltip does not display at all. My rationale was to tackle the problem at the root rather than to work around it by splitting the string within baloo-widgets. With regard to T8079 <https://phabricator.kde.org/T8079> it will be the same. If data is taken from baloo's database that old long string will be displayed
  
  I am sure that your work is really trying to fix real problem that are seeing. You have my full support on your aim.
  
  I should apologize given how bad my knowledge of KFileMetaData is. It is already silently managing single and multiple entries. In fact, yes, the epub extractor is doing it wrong.
  I have also checked how baloo is handling multiple entries and it works in the same way than pure kfilemetadata.
  
  In the taglib extractors multiple entries are added in several call to add method. This is nicely handled by KFileMetaData::SimpleExtractionResult::add . It would be nice if you modify your patch to follow the same approach. In Baloo, things are a little bit different and a list is automatically created when multiple add are called with the same property but the result is similar (in baloo Result::add ).
  
  You can forget my initial objection, I was plain wrong.
  
  We may have bugs in user of the API not expecting a list when they should have. In my opinion, this should not block your diff. Let's fix them when we discover them.
  
  > According to the standard an epub can have multiple `dc:subject` entries. So it is reasonable to expect a string list from KFileMetaData. Plus the possibilities are grand with an KFileMetaData writer for epubs one could use the tag widget to change them. One could unify xattr tags and epub subjects in tags:// protocol and mayby more.

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/20180227/b2faebe2/attachment-0001.html>