Re: Moving KFIleMetadata into KDE SC
On Thursday 23 January 2014 11:20:54 you wrote: > On Wednesday 22 January 2014 19:15:01 Àlex Fiestas wrote: > > · I would add the url and mimetype in the cto, plugins should not be able > > to change it anyway. > > Fixed > > > · Documentation needs improvement, specially ExtractionResult (making it > > clear that you should inherit it in order to use the lib). > > Fixed. > > > · ExtratorPlugin::mimetypes is pure virtual yet it has an implementation > > Fixed. > > > · Add context to i18n in typeinfo > > Fixed > > > Also you might want to use Qt translation libs so kfilemetadata becomes a > > Qt only lib, that might help other desktops/apps to use it (in kf5 you > > can use k18n if you want). > > Sticking with i18n for now. > > > Once this is fix, yo get +1 from me ! > > Thanks' Awesome! good to go then to the "virtual" kdelibs :)
Re: Moving KFIleMetadata into KDE SC
On Wednesday 22 January 2014 19:15:01 Àlex Fiestas wrote: > · I would add the url and mimetype in the cto, plugins should not be able to > change it anyway. Fixed > · Documentation needs improvement, specially ExtractionResult (making it > clear that you should inherit it in order to use the lib). Fixed. > · ExtratorPlugin::mimetypes is pure virtual yet it has an implementation Fixed. > · Add context to i18n in typeinfo Fixed > Also you might want to use Qt translation libs so kfilemetadata becomes a Qt > only lib, that might help other desktops/apps to use it (in kf5 you can use > k18n if you want). Sticking with i18n for now. > > Once this is fix, yo get +1 from me ! Thanks -- Vishesh Handa
Re: Moving KFIleMetadata into KDE SC (documentation and ExtractionResult)
El Dimecres, 22 de gener de 2014, a les 18:47:37, Vishesh Handa va escriure: > On Wednesday 22 January 2014 13:09:58 David Edmundson wrote: > > Add COPYING file > > Fixed > > > > > ExtractorPluginManager::fetchExtractors seems odd to me. > > If it can't find any plugins it searches for all plugins that start > > the same prefix. > > > > I assume it's designed so I can have a plugin with the mimetype audio/ > > that will still match the file mimetype audio/mp3 > > But this means that if I add a special new plugin with the mimetype > > audio/mp3 the original plugin will stop running? Given you're trying > > to build a list of all valid plugins, is that meant to happen? > > Yes. > > I've added the relevant documentation. > > > -- > > > > ExtractionResult: > > > > Having setInputUrl / setInputMimetype public in a way which can be set > > by the plugins seems wrong; personally I'd put the two in the > > constructor. > > Fixed > > > -- > > > > We should document the list of names of properties that can be extracted. > > Some things are in lowerCamelCase i.e "wordCount" "author" except for > > the eviv plugin which is in the form: > > Exif.Image.Make > > > > and the odf extractor which is in the form > > > > "dc:title" > > > > to me it seems fairly random. > > Fixed. Added a proper property and type system. Looks good to me. +1 to move to kdelibs. Cheers, Albert
Re: Moving KFIleMetadata into KDE SC
El Dimecres, 22 de gener de 2014, a les 19:15:01, Àlex Fiestas va escriure: > Also you might want to use Qt translation libs so kfilemetadata becomes a Qt > only lib, that might help other desktops/apps to use it (in kf5 you can use > k18n if you want). Do not do that at the moment please, this increases the complexity of making l10n work properly for us and it may just be better to wait for the kf5 version to do that. Cheers, Albert
Re: Moving KFIleMetadata into KDE SC
· I would add the url and mimetype in the cto, plugins should not be able to change it anyway. · Documentation needs improvement, specially ExtractionResult (making it clear that you should inherit it in order to use the lib). · ExtratorPlugin::mimetypes is pure virtual yet it has an implementation · Add context to i18n in typeinfo Also you might want to use Qt translation libs so kfilemetadata becomes a Qt only lib, that might help other desktops/apps to use it (in kf5 you can use k18n if you want). Once this is fix, yo get +1 from me !
Re: Moving KFIleMetadata into KDE SC (documentation and ExtractionResult)
On Wednesday 22 January 2014 13:09:58 David Edmundson wrote: > Add COPYING file Fixed > > ExtractorPluginManager::fetchExtractors seems odd to me. > If it can't find any plugins it searches for all plugins that start > the same prefix. > > I assume it's designed so I can have a plugin with the mimetype audio/ > that will still match the file mimetype audio/mp3 > But this means that if I add a special new plugin with the mimetype > audio/mp3 the original plugin will stop running? Given you're trying > to build a list of all valid plugins, is that meant to happen? > Yes. I've added the relevant documentation. > -- > > ExtractionResult: > > Having setInputUrl / setInputMimetype public in a way which can be set > by the plugins seems wrong; personally I'd put the two in the > constructor. > Fixed > > -- > > We should document the list of names of properties that can be extracted. > Some things are in lowerCamelCase i.e "wordCount" "author" except for > the eviv plugin which is in the form: > Exif.Image.Make > > and the odf extractor which is in the form > > "dc:title" > > to me it seems fairly random. Fixed. Added a proper property and type system. -- Vishesh Handa
Re: Moving KFIleMetadata into KDE SC (documentation and ExtractionResult)
Add COPYING file ExtractorPluginManager::fetchExtractors seems odd to me. If it can't find any plugins it searches for all plugins that start the same prefix. I assume it's designed so I can have a plugin with the mimetype audio/ that will still match the file mimetype audio/mp3 But this means that if I add a special new plugin with the mimetype audio/mp3 the original plugin will stop running? Given you're trying to build a list of all valid plugins, is that meant to happen? -- ExtractionResult: Having setInputUrl / setInputMimetype public in a way which can be set by the plugins seems wrong; personally I'd put the two in the constructor. -- We should document the list of names of properties that can be extracted. Some things are in lowerCamelCase i.e "wordCount" "author" except for the eviv plugin which is in the form: Exif.Image.Make and the odf extractor which is in the form "dc:title" to me it seems fairly random.
Re: Moving KFIleMetadata into KDE SC
On Wednesday 22 January 2014 00:14:27 Albert Astals Cid wrote: > El Dimarts, 21 de gener de 2014, a les 11:56:11, Vishesh Handa va escriure: > > On Monday 20 January 2014 22:37:12 Albert Astals Cid wrote: > > > Please have a look at the const-ness of the methods, there's lots of > > > "geters" that are non const. > > > > Fixed > > Do you think > virtual QStringList mimetypes() = 0; > > May be better as const? > Hmm, perhaps. Fixed. -- Vishesh Handa
Re: Moving KFIleMetadata into KDE SC (documentation and ExtractionResult)
On Wednesday 22 January 2014 00:23:45 Albert Astals Cid wrote:
> El Divendres, 17 de gener de 2014, a les 18:03:31, Vishesh Handa va
escriure:
> > Hey guys
> >
> > I should have posted this with the Baloo thread, but since I did not -
> >
> > WIth KDE SC 4.10, Nepomuk dropped support for Strigi and implemented their
> > own indexing library. This code was part of nepomuk-core. With Baloo, this
> > code has been migrated into its own repository called kfilemetadata. I'm
> > hoping to make it into its own framework some point in the future.
> >
> > I would like to move this code into KDE SC along with Baloo.
> >
> > None of the code is new. It has all been taken from neopmuk-core. The only
> > change is that the indexers no longer use the ontologies.
>
> Can you have a look at
> virtual void extract(ExtractionResult* result) = 0;
> It seems to me that the documentation is wrong, it lists lots of params.
>
Fixed.
> Also i tried looking at ExtractionResult and without documentation i'm not
> sure i'd be able to use it, could you add some docu and/or example?
>
I've improved the documentation to a certain extent. I'll try adding an actual
example later. Maybe once I've figured out the property system (see below)
> Also I was wondering what is if we could move from all those strings to
> enums?
>
> I mean
> result->add("copyright", it.value());
> is a bit hard, since whatever reads the result has to know it has to query
> for "copyright".
I've been trying to build a property system which would incorporate -
1. Actual names instead of strings
2. Property ranges. Eg - height should be an integer
3. Property name translations - The way translations are currently handled is
ugly. It works, but it is ugly.
4. Should the property be globally indexed - Some properties should not
globally indexed, and should only appear when the user searches for that exact
property.
RDF provided a nice way to specifying all of this, but I'm not too keen on
using RDF. In the end, I may just use enums, but for now strings it is.
>
> Also i am wondering why addType is virtual and append/add are pure virtual.
>
I thought the derived classes may not care about the type. I've changed it to
pure virtual to be more consistent.
> Cheers,
> Albert
Thanks for looking into this Albert.
--
Vishesh Handa
Re: Moving KFIleMetadata into KDE SC
On Wednesday 22 January 2014 00:16:05 Albert Astals Cid wrote: > El Dimarts, 21 de gener de 2014, a les 11:56:33, Vishesh Handa va escriure: > > On Monday 20 January 2014 22:38:52 Albert Astals Cid wrote: > > > Also make sure all the classes have a d-pointer in case they need to be > > > expanded in the future you can keep ABI. > > > > Fixed. > > Maybe even add one to ExtractorPlugin in case you need to add stuff in there > later? > I didn't think I would need it in the future, but you're right. It's better to be safe. > What about moving > ExtractorPluginManager::allExtractors to the private? > Fixed. -- Vishesh Handa
Re: Moving KFIleMetadata into KDE SC (documentation and ExtractionResult)
El Divendres, 17 de gener de 2014, a les 18:03:31, Vishesh Handa va escriure:
> Hey guys
>
> I should have posted this with the Baloo thread, but since I did not -
>
> WIth KDE SC 4.10, Nepomuk dropped support for Strigi and implemented their
> own indexing library. This code was part of nepomuk-core. With Baloo, this
> code has been migrated into its own repository called kfilemetadata. I'm
> hoping to make it into its own framework some point in the future.
>
> I would like to move this code into KDE SC along with Baloo.
>
> None of the code is new. It has all been taken from neopmuk-core. The only
> change is that the indexers no longer use the ontologies.
Can you have a look at
virtual void extract(ExtractionResult* result) = 0;
It seems to me that the documentation is wrong, it lists lots of params.
Also i tried looking at ExtractionResult and without documentation i'm not
sure i'd be able to use it, could you add some docu and/or example?
Also I was wondering what is if we could move from all those strings to enums?
I mean
result->add("copyright", it.value());
is a bit hard, since whatever reads the result has to know it has to query for
"copyright".
Also i am wondering why addType is virtual and append/add are pure virtual.
Cheers,
Albert
>
> --
> Vishesh Handa
Re: Moving KFIleMetadata into KDE SC
El Dimarts, 21 de gener de 2014, a les 11:56:33, Vishesh Handa va escriure: > On Monday 20 January 2014 22:38:52 Albert Astals Cid wrote: > > Also make sure all the classes have a d-pointer in case they need to be > > expanded in the future you can keep ABI. > > Fixed. Maybe even add one to ExtractorPlugin in case you need to add stuff in there later? What about moving ExtractorPluginManager::allExtractors to the private? Cheers, Albert
Re: Moving KFIleMetadata into KDE SC
El Dimarts, 21 de gener de 2014, a les 11:56:11, Vishesh Handa va escriure: > On Monday 20 January 2014 22:37:12 Albert Astals Cid wrote: > > Please have a look at the const-ness of the methods, there's lots of > > "geters" that are non const. > > Fixed Do you think virtual QStringList mimetypes() = 0; May be better as const? Cheers, Albert
Re: Moving KFIleMetadata into KDE SC
On Monday 20 January 2014 22:38:52 Albert Astals Cid wrote: > > Also make sure all the classes have a d-pointer in case they need to be > expanded in the future you can keep ABI. > Fixed. -- Vishesh Handa
Re: Moving KFIleMetadata into KDE SC
On Monday 20 January 2014 22:37:12 Albert Astals Cid wrote: > > Please have a look at the const-ness of the methods, there's lots of > "geters" that are non const. Fixed -- Vishesh Handa
Re: Moving KFIleMetadata into KDE SC
On Monday 20 January 2014 22:39:30 Albert Astals Cid wrote: > El Divendres, 17 de gener de 2014, a les 18:03:31, Vishesh Handa va escriure: > > Hey guys > > > > I should have posted this with the Baloo thread, but since I did not - > > > > WIth KDE SC 4.10, Nepomuk dropped support for Strigi and implemented their > > own indexing library. This code was part of nepomuk-core. With Baloo, this > > code has been migrated into its own repository called kfilemetadata. I'm > > hoping to make it into its own framework some point in the future. > > > > I would like to move this code into KDE SC along with Baloo. > > Forgot to ask, which module would that be part of? kde-runtime? Or? > To be clear - I mean under the KDE Libraries. The same place where nepomuk- core, nepomuk-widgets, and activites are. > Cheers, > Albert > > > None of the code is new. It has all been taken from neopmuk-core. The only > > change is that the indexers no longer use the ontologies. > > > > -- > > Vishesh Handa -- Vishesh Handa
Re: Moving KFIleMetadata into KDE SC
On Monday 20 January 2014 22:39:30 Albert Astals Cid wrote: > El Divendres, 17 de gener de 2014, a les 18:03:31, Vishesh Handa va escriure: > > Hey guys > > > > I should have posted this with the Baloo thread, but since I did not - > > > > WIth KDE SC 4.10, Nepomuk dropped support for Strigi and implemented their > > own indexing library. This code was part of nepomuk-core. With Baloo, this > > code has been migrated into its own repository called kfilemetadata. I'm > > hoping to make it into its own framework some point in the future. > > > > I would like to move this code into KDE SC along with Baloo. > > Forgot to ask, which module would that be part of? kde-runtime? Or? > kdelibs > Cheers, > Albert > > > None of the code is new. It has all been taken from neopmuk-core. The only > > change is that the indexers no longer use the ontologies. > > > > -- > > Vishesh Handa -- Vishesh Handa
Re: Moving KFIleMetadata into KDE SC
El Divendres, 17 de gener de 2014, a les 18:03:31, Vishesh Handa va escriure: > Hey guys > > I should have posted this with the Baloo thread, but since I did not - > > WIth KDE SC 4.10, Nepomuk dropped support for Strigi and implemented their > own indexing library. This code was part of nepomuk-core. With Baloo, this > code has been migrated into its own repository called kfilemetadata. I'm > hoping to make it into its own framework some point in the future. > > I would like to move this code into KDE SC along with Baloo. Forgot to ask, which module would that be part of? kde-runtime? Or? Cheers, Albert > > None of the code is new. It has all been taken from neopmuk-core. The only > change is that the indexers no longer use the ontologies. > > -- > Vishesh Handa
Re: Moving KFIleMetadata into KDE SC
El Divendres, 17 de gener de 2014, a les 18:03:31, Vishesh Handa va escriure: > Hey guys > > I should have posted this with the Baloo thread, but since I did not - > > WIth KDE SC 4.10, Nepomuk dropped support for Strigi and implemented their > own indexing library. This code was part of nepomuk-core. With Baloo, this > code has been migrated into its own repository called kfilemetadata. I'm > hoping to make it into its own framework some point in the future. > > I would like to move this code into KDE SC along with Baloo. Also make sure all the classes have a d-pointer in case they need to be expanded in the future you can keep ABI. Cheers, Albert > > None of the code is new. It has all been taken from neopmuk-core. The only > change is that the indexers no longer use the ontologies. > > -- > Vishesh Handa
Re: Moving KFIleMetadata into KDE SC
El Divendres, 17 de gener de 2014, a les 18:03:31, Vishesh Handa va escriure: > Hey guys > > I should have posted this with the Baloo thread, but since I did not - > > WIth KDE SC 4.10, Nepomuk dropped support for Strigi and implemented their > own indexing library. This code was part of nepomuk-core. With Baloo, this > code has been migrated into its own repository called kfilemetadata. I'm > hoping to make it into its own framework some point in the future. > > I would like to move this code into KDE SC along with Baloo. > > None of the code is new. It has all been taken from neopmuk-core. The only > change is that the indexers no longer use the ontologies. Please have a look at the const-ness of the methods, there's lots of "geters" that are non const. Cheers, Albert > > -- > Vishesh Handa
