D8347: KIO::PreviewJob::defaultPlugins() function

Mark Gaiser noreply at phabricator.kde.org
Wed Oct 18 18:06:54 BST 2017


markg accepted this revision.
markg added a comment.
This revision is now accepted and ready to land.


  In https://phabricator.kde.org/D8347#156930, @ngraham wrote:
  
  > I still need to add a @since in the header documentation, after which point hopefully @markg will approve. :)
  
  
  +1 then. I trust you do that.
  Also note that a proper container for what you're doing is two sets (default plugins and the blacklisted ones) and calling the std::set_difference algorithm on them; see: https://www.fluentcpp.com/2017/01/09/know-your-algorithms-algos-on-sets/
  That's just minor details though, feel free to ignore that :)
  
  I remain skeptical of your entire goal here though. It's good and well intended, that's for sure. But ultimately this is for Dolphin and the previews which in turn means that by default all but one plugin is going to be enabled. I'm skeptical about that because there is always an issue with doing that. Either bad plugins causing weird behavior, bad data (just plain and simple broken data) causing weird behavior. And probably a gazillion other reasons once it is enabled and used in the next dolphin version...
  
  I hope you're up for a lot of bug reports :D
  
  On the other hand, it would be very cool if this starts working properly!

REPOSITORY
  R241 KIO

BRANCH
  master

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

To: ngraham, #frameworks, broulik, #dolphin, markg
Cc: markg, anthonyfieroni, elvisangelaccio, #frameworks
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20171018/126a6da0/attachment.htm>


More information about the kfm-devel mailing list