Making KFileMetaData a framework

[Vishesh Handa]

On Tuesday, August 05, 2014 07:58:03 PM David Edmundson wrote:
> In general that's some of the tidied code I've seen in a long time.

Thanks!

> 
> Comments below. One major, most not.
> 
> ----
> TypeInfo/PropertyInfo/SimpleExtractionResult need a working &operator=
> otherwise we shallow copy d.
> 
> I can cause a crash in dump.cpp

Fixed

> ----
> 
> Extracting info from a file can take a long time, right now you spawn
> a separate execuatble for Baloo; but for other uses (i.e dolphin
> getting info on a non-indexed file) you might want to add a helper
> API; either a new thread or a service like baloo-file-extractor and a
> wrapper round calling it.
> 
> ----
> 
> ExtractorPluginManager::Private::allExtractors
> what's going on with the variable "plugins"? You're always checking if
> an empty list contains things. Looks leftover from something.
> 

Fixed

> ----
> 
> In properties.h I would space out the enum so that in the future when
> you insert extra ones you can put it alongside the relevant category
> without breaking API
> 
> i.e
> //Audio
> BitRate = 100,
> Channels,
> Duration,
> 
> // Documents
> Author =200,
> Title,
> ...
> 
> Otherwise it'll be an ungrouped mess in future releases if you ever
> have to add another audio property.
> 

Might make sense. Currently adding gaps in the properties would break my 
tests, but it might be a good idea. I'll look more into it.

> -----
> ExtractionPluginManager possibly shouldn't be a QObject?

Fixed

> 
> It /might/ be a good idea to make ExtractionPluginManager static so
> you don't load the plugins every time (which can be a bit expensive)
> 

Hmm, Possibly.

> 
> ----
> 
> dump.cpp should check there's at least one arg.
> (I know it's a test, but you explicitly check for >=2 but not <1)
> 

Fixed 

Thanks for reviewing the code.

-- 
Vishesh Handa