Review Request: Add KMetaDataWidget and KMetaDataConfigurationDialog to kdelibs/kfile

Peter Penz peter.penz at gmx.at
Tue Oct 27 11:23:31 GMT 2009 


> On 2009-10-27 08:30:00, Sebastian Trueg wrote:
> > trunk/KDE/kdelibs/kfile/kmetadatawidget.h, line 54
> > <http://reviewboard.kde.org/r/1938/diff/2/?file=13409#file13409line54>
> >
> >     While tags, ratings, and comments are only 3 types of Nepomuk data, IMHO it makes sense to have them in here. After all these are the "classical" meta data types we have since KDE 4.0.
> >     However, I think we should add something like "NepomukData" to complete the list. This would allow to disable any meta data which is in Nepomuk.
> >     Or maybe even a distinction between the actual file meta data as extractes by Strigi and manual annotations.

This is a tricky topic to discuss... It generally raises the question whether the enabling/disabling of meta data is application specific or whether it is a global configuration that can be adjusted by the user. Initially I was convinced that it is a global configuration and provided KMetaDataConfigurationDialog which allows the user to enable/disable meta data to his needs. The benefit of this approach is that there is no API needed in KMetaDataWidget which requires applications to deal with hundreds of meta data types.

But Tom Albers wanted to disable the showing of some meta data especially for Mailody and I think this is a valid usecase (e. g. "rating" for e-mails might not be required). This raises the problem how to map the hundreds of possible meta data types in an easy way to applications, so that they can disable it. Providing just the internal (I say it a little bit clumsy now:) "Nepomuk-string" requires the application to have a lot of knowledge what string must be passed to e. g. turn off the "rating". So I decided to provide just a set of enums which allows to turn off the (what I call) "fixed meta data": = Meta data that is shown for each file even the value is empty (e. g. like it might be the case for rating, comments and tags). I'm not really convinced that this is the best possible approach and I'd appreciate it if somebody could came up with a better interface/approach.

| I think we should add something like "NepomukData" to complete the list.
| This would allow to disable any meta data which is in Nepomuk.

This is possible for the user already by KMetaDataConfigurationDialog. But it is not possible currently for applications to individually turn off specific meta data which is in Nepomuk. Again this raises the question whether we require an application specific setup for KMetaDataWidget...


> On 2009-10-27 08:30:00, Sebastian Trueg wrote:
> > trunk/KDE/kdelibs/kfile/kmetadatawidget.h, line 72
> > <http://reviewboard.kde.org/r/1938/diff/2/?file=13409#file13409line72>
> >
> >     Do we actually need this struct. I always find structs like this in public API a bit dodgy.
> >     Maybe it would be better to only use the struct internally and expose methods like
> >     "addCustomData( QString label, QString value )"
> >     Or even "addCustomData( QString label, QVariant value )"

Hm, I agree: the struct is a bit dodgy ;-) Considering that the custom data must be adjusted each time the KMetaDataWidget::setItem() methods are called, _adding_ a label might be problematic (-> we'd also require a removeCustomData() method etc.). Maybe we can stick with: void setCustomData(int key, const QString& label, const QString& value)?

The more we discuss this topic (dynamic vs. fixed vs. custom meta data), the more I've the impression that we should not rush to get the KMetaDataWidget into KDE 4.4 already :-/ I like the approach Aurélien mentions below, but I don't have the time that I can do it until KDE 4.4...
 


- Peter


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/1938/#review2821
-----------------------------------------------------------


On 2009-10-27 08:06:19, Peter Penz wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/1938/
> -----------------------------------------------------------
> 
> (Updated 2009-10-27 08:06:19)
> 
> 
> Review request for kdelibs, Sebastian Trueg and David Faure.
> 
> 
> Summary
> -------
> 
> The patch adds KMetaDataWidget and KMetaDataConfigurationDialog as public classes to kdelibs/kfile. KMetaDataWidget allows an application in an easy way to show meta data of a file (or several files). The widget also allows to change meta data like tags, comments and rating: http://enzosworld.gmxhome.de/temp/metadatawidget_new.png KMetaDataConfigurationDialog allows to configure which meta tags should be hidden/shown. The classes also work without Nepomuk (and show only very basic meta data like size, permissions, ...).
> 
> The classes have been used by Dolphin internally until now and have originally been written by Sebastian Trüg. After the request from Tom Albers and Oliver Heidbüchel to integrate the widget also in Mailody/Okular I've adjusted the classes to get them ready for a kdelibs-integration.
> 
> Sebastian Trüg, Tom Albers, Oliver Heidbüchel and I did already an internal review and the classes have been tested in the context of Mailody and Okular. There are still some minor implementation issues, but the main reason for this request is to review the public API and the integration into kdelibs/kfile (I'll take care to fix the issues until KDE 4.4).
> 
> I'd ask to mainly look at the files kfile/kmetadatawidget.h, kfile/kmetadataconfigurationdialog.h and kfile/CMakeLists.txt One ugly hack in the header file is the HAVE_NEPOMUK part in kmetadatawidget.h. As Nepomuk runs with Virtuoso now, the chances are good that we can get rid of this hack until KDE 4.4 (see also http://lists.kde.org/?l=kde-core-devel&m=125577498913008&w=2 for the discussion on kde-core-devel).
> 
> Please let me know whether there are general concerns regarding the location or the HAVE_NEPOMUK issue.
> 
> 
> Diffs
> -----
> 
>   trunk/KDE/kdelibs/kfile/CMakeLists.txt 1038666 
>   trunk/KDE/kdelibs/kfile/kcommentwidget.cpp PRE-CREATION 
>   trunk/KDE/kdelibs/kfile/kcommentwidget_p.h PRE-CREATION 
>   trunk/KDE/kdelibs/kfile/kedittagsdialog.cpp PRE-CREATION 
>   trunk/KDE/kdelibs/kfile/kedittagsdialog_p.h PRE-CREATION 
>   trunk/KDE/kdelibs/kfile/kmetadataconfigurationdialog.h PRE-CREATION 
>   trunk/KDE/kdelibs/kfile/kmetadataconfigurationdialog.cpp PRE-CREATION 
>   trunk/KDE/kdelibs/kfile/kmetadatawidget.h PRE-CREATION 
>   trunk/KDE/kdelibs/kfile/kmetadatawidget.cpp PRE-CREATION 
>   trunk/KDE/kdelibs/kfile/ktaggingwidget.cpp PRE-CREATION 
>   trunk/KDE/kdelibs/kfile/ktaggingwidget_p.h PRE-CREATION 
>   trunk/KDE/kdelibs/nepomuk/core/ui/CMakeLists.txt 1038666 
>   trunk/KDE/kdelibs/nepomuk/core/ui/nepomukmassupdatejob.h 1038666 
>   trunk/KDE/kdelibs/nepomuk/core/ui/nepomukmassupdatejob.cpp 1038666 
> 
> Diff: http://reviewboard.kde.org/r/1938/diff
> 
> 
> Testing
> -------
> 
> Tested in Dolphin, Mailody and Okular. Some minor implementation issues are open, but the interface seems to be sufficient.
> 
> 
> Thanks,
> 
> Peter
> 
>

More information about the kde-core-devel mailing list