Review Request: Add KMetaDataWidget, KMetaDataModel and KMetaDataConfigurationDialog

Mon Mar 22 21:11:32 GMT 2010

> On 2010-03-21 16:29:31, Aurélien Gâteau wrote:
> > I didn't look at individual lines closely as others have already did so. I think there was a misunderstanding regarding my suggestion on the initial request (as far as I remember what I suggested...). I think i suggested it would be more useful to have a widget to display a set of properties, but what I meant was that these properties do not have to be properties of a file. One could imagine for example a calendar showing the properties of a meeting.
> > Since KMetaDataWidget is tied to files, I agree KMetaDataModel can be made private, at least for now. I must confess I also agree with Christopher about the name being a bit confusing. Even if, as Peter says "Qt has no ownership on the MVC pattern", I think Qt users have come to expect that classes suffixed with "Model" inherits from QAbstractItemModel or a subclass of it, in the same way that classes suffixed with "Widget" are expected to inherit from QWidget.
> > 
> > One question though: I don't see the connection between KMetaDataWidget and KMetaDataConfigurationDialog. When does one create such a dialog? How does one link it to KMetaDataWidget?
> 
> Peter Penz wrote:
>     As KMetaDataModel is a private API now, it won't be a problem to rename it. I'm open for any suggestions, I somehow would like the name KMetaDataProvider as Christoph said... What do you think?
>     
>     Regarding KMetaDataWidget and KMetaDataConfigurationDialog: You can take Dolphin as example. If you right click on the Information Panel and select "Configure...", you can configure which of the meta data for the currently shown file type should be shown at all. IMO this should not be possible only within Dolphin, but also in other applications that might use this widget.
>     
>
> 
> Aurélien Gâteau wrote:
>     KMetaDataProvider sounds good.
>     
>     About the dialog: what I was wondering was how was the developer supposed to "link" the dialog and the widget. Looking at the code I realized they communicate via the "Nepomuk KMetaDataConfigDialog" configuration group. Should the group be hardcoded? I don't really know. Are there other places in kdelibs with hardcoded config groups like this?
> 
> Sebastian Trueg wrote:
>     I think I have to agree with Aurelien as far as the widget is concerned. Why don't we call it KFileMetaDataWidget to keep the possibility open to add a generic KMetaDataWidget later which could then internally be used by the former.
>     
>     About the dialog: I don't think a dialog is the best way to go. Other apps might want to embed the config in some other place. So ideally at some point we would have a data model and a filter model which has configuration methods to hide/show specific information bits. Since the model is not public yet I also vote to hide the dialog/config for now. Then we figure out how to configure the model. I know that this is taking the easy way out but as this is already the second attempt at getting the class into kdelibs it could make sense to do it step by step.
> 
> Peter Penz wrote:
>     OK, I'll rename the widget to KFileMetaDataWidget. Regarding the dialog: The only thing I can do is adding a @internal to the complete class, but it is not possible for me to move the dialog from kdelibs to Dolphin, as the meta data model from kdelibs is private. Would this (= adding an @internal to the class) be acceptable?
>     
>     If possible it would be great having an answer until today 0:00, so that I don't have to wait another week for the commit. Thanks!

About the dialog: maybe you can turn it into a widget? This way applications can embed it in a configuration dialog.

- Aurélien

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

On 2010-03-22 18:02:40, Peter Penz wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/3277/
> -----------------------------------------------------------
> 
> (Updated 2010-03-22 18:02:40)
> 
> 
> Review request for kdelibs, Sebastian Trueg, David Faure, and Aurélien Gâteau.
> 
> 
> Summary
> -------
> 
> The patch adds KMetaDataWidget, KMetaDataModel and KMetaDataConfigurationDialog as public classes to kdelibs/kfile. The 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 (see http://enzosworld.gmxhome.de/temp/metadatawidget.png or attached screenshot). 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, ...). It is possible for applications to add custom meta data if wanted (including widgets to manipulate this meta data).
> 
> 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. I'd also like to to adjust KPropertiesDialog later to use this widget.
> 
> I'd ask mainly to look at the files kfile/kmetadatawidget.h, kfile/kmetadatamodel.h and kfile/kmetadataconfigurationdialog.h, the other APIs are internal.
> 
> Thanks!
> 
> 
> Diffs
> -----
> 
>   trunk/KDE/kdelibs/CMakeLists.txt 1106199 
>   trunk/KDE/kdelibs/config-nepomuk.h.cmake PRE-CREATION 
>   trunk/KDE/kdelibs/kfile/CMakeLists.txt 1106199 
>   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/kfilemetadataconfigurationdialog.h PRE-CREATION 
>   trunk/KDE/kdelibs/kfile/kfilemetadataconfigurationdialog.cpp PRE-CREATION 
>   trunk/KDE/kdelibs/kfile/kfilemetadataprovider.cpp PRE-CREATION 
>   trunk/KDE/kdelibs/kfile/kfilemetadataprovider_p.h PRE-CREATION 
>   trunk/KDE/kdelibs/kfile/kfilemetadatawidget.h PRE-CREATION 
>   trunk/KDE/kdelibs/kfile/kfilemetadatawidget.cpp PRE-CREATION 
>   trunk/KDE/kdelibs/kfile/kloadfilemetadatathread.cpp PRE-CREATION 
>   trunk/KDE/kdelibs/kfile/kloadfilemetadatathread_p.h PRE-CREATION 
>   trunk/KDE/kdelibs/kfile/knfotranslator.cpp PRE-CREATION 
>   trunk/KDE/kdelibs/kfile/knfotranslator_p.h 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 1106199 
>   trunk/KDE/kdelibs/nepomuk/core/ui/nepomukmassupdatejob.h 1106199 
>   trunk/KDE/kdelibs/nepomuk/core/ui/nepomukmassupdatejob.cpp 1106199 
> 
> Diff: http://reviewboard.kde.org/r/3277/diff
> 
> 
> Testing
> -------
> 
> Tested in Dolphin. An early version has been tested also in Mailody and Okular.
> 
> 
> Screenshots
> -----------
> 
> KMetaDataWidget
>   http://reviewboard.kde.org/r/3277/s/330/
> 
> 
> Thanks,
> 
> Peter
> 
>