Review Request: Add KMetaDataWidget, KMetaDataModel and KMetaDataConfigurationDialog

Peter Penz peter.penz at gmx.at
Fri Mar 19 21:35:14 GMT 2010 


> On 2010-03-19 10:00:17, Sebastian Trueg wrote:
> > trunk/KDE/kdelibs/kfile/kmetadatamodel.h, line 49
> > <http://reviewboard.kde.org/r/3277/diff/2/?file=20853#file20853line49>
> >
> >     IMHO this file should not be made public yet. I doubt anyone will reimplement this model soon. And the less API is public the more we can tune it afterwards.

It was mainly intended for Gwenview. Aurélien was interested into adding custom meta data. But like above I think it is a good idea to make this a private interface in the meantime. Aurélien could still use it and we could finetune the interface until it gets public.


> On 2010-03-19 10:00:17, Sebastian Trueg wrote:
> > trunk/KDE/kdelibs/kfile/kmetadataconfigurationdialog.h, line 34
> > <http://reviewboard.kde.org/r/3277/diff/2/?file=20851#file20851line34>
> >
> >     Like with the model I would not make this public yet.

Hm... I think you are right. The KPropertiesDialog won't use this class and even if applications like Gwenview or Okular will use the KMetaDataWidget, it is unclear yet whether they need the configuration dialog in this way. OK, so I'll leave the KMetaDataConfiguration in Dolphin now. In the worst case, if other applications will start to copy this class, we might think again about moving it to kdelibs later after getting feedback from the app-developers.


> On 2010-03-19 10:00:17, Sebastian Trueg wrote:
> > trunk/KDE/kdelibs/kfile/kmetadatawidget.h, line 134
> > <http://reviewboard.kde.org/r/3277/diff/2/?file=20855#file20855line134>
> >
> >     This could be moved into the model to keep any URI stuff out of the widget. If you want to keep Nepomuk as an option using URIs as property identifier is probably not best anyway.

Although Nepomuk is optional, having mixed kind of keys for meta data is worse I think (from an interface point of view and implementation point of view). If someone adds meta data, where no official URI is defined, it is no problem to define a custom URI. But yes: This should be moved to the model (I'll change this - I initially thought the translation should belong to the widget, but the labels just belong to the meta data of the model).


> On 2010-03-19 10:00:17, Sebastian Trueg wrote:
> > trunk/KDE/kdelibs/kfile/kmetadatawidget.h, line 148
> > <http://reviewboard.kde.org/r/3277/diff/2/?file=20855#file20855line148>
> >
> >     Why not move this into the model, too. Then we can optimize it in any way until we make the model public.
> >     Or if you think a widget method should not be in the model just make it non-public for now to keep the API as clean as possible. 
> >     Any further improvements can be done later. And I don't think any client does override custom widgets at this point.

I'll move those 2 methods into the model, with minor adjustments (valueWidget() must get a factory method...). As you say it gives us more chances to work on improvements without thinking about API backward compatibility.


> On 2010-03-19 10:00:17, Sebastian Trueg wrote:
> > trunk/KDE/kdelibs/kfile/kmetadatawidget.h, line 157
> > <http://reviewboard.kde.org/r/3277/diff/2/?file=20855#file20855line157>
> >
> >     The simplest solution here would be to use QVariant. After all we can easily put a Nepomuk::Variant in a QVariant or just convert it internally.

I'll check this. Thanks for your good suggestions!


- Peter


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


On 2010-03-16 20:39:25, Peter Penz wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/3277/
> -----------------------------------------------------------
> 
> (Updated 2010-03-16 20:39:25)
> 
> 
> 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 1103259 
>   trunk/KDE/kdelibs/config-nepomuk.h.cmake PRE-CREATION 
>   trunk/KDE/kdelibs/kfile/CMakeLists.txt 1103259 
>   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/kloadmetadatathread.cpp PRE-CREATION 
>   trunk/KDE/kdelibs/kfile/kloadmetadatathread_p.h PRE-CREATION 
>   trunk/KDE/kdelibs/kfile/kmetadataconfigurationdialog.h PRE-CREATION 
>   trunk/KDE/kdelibs/kfile/kmetadataconfigurationdialog.cpp PRE-CREATION 
>   trunk/KDE/kdelibs/kfile/kmetadatamodel.h PRE-CREATION 
>   trunk/KDE/kdelibs/kfile/kmetadatamodel.cpp PRE-CREATION 
>   trunk/KDE/kdelibs/kfile/kmetadatawidget.h PRE-CREATION 
>   trunk/KDE/kdelibs/kfile/kmetadatawidget.cpp 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 1103259 
>   trunk/KDE/kdelibs/nepomuk/core/ui/nepomukmassupdatejob.h 1103259 
>   trunk/KDE/kdelibs/nepomuk/core/ui/nepomukmassupdatejob.cpp 1103259 
> 
> 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
> 
>

More information about the kde-core-devel mailing list