Review Request: Add KMetaDataWidget and KMetaDataConfigurationDialog to kdelibs/kfile

[nf2]

On Fri, Oct 23, 2009 at 11:07 AM, Peter Penz <peter.penz at gmx.at> wrote:
>>
>> 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.

As i'm dealing with this kind of "everything depends on everything" in
KDE problem with KFilePlacesModel - which is not ideal in terms of
portablility and modularity i guess...

The problem here seems to be the public class KFILE_EXPORT
KMetaDataWidget, which introduces an dependency to a nepumuk type at
two places:

QList<Nepomuk::Tag> tags() const;

void tagsChanged(const QList<Nepomuk::Tag>& tags);

I guess to solve this issue in OO fashion, wouldn't be #ifdefs, but
making KMetaDataWidget castable to a sub-class or interface, which
really depends on Nepomuk (but doesn't sit in kfile). A second option
would be to somehow wrap the Nepomuk::Tags leaking out of the API in a
way that they not necessarily have to be Nepomuk::Tags. The problem
here is the Nepomuk::Tag is not an object type, otherwise in the lists
would just be List<Object>, which could be casted to Nepomuk::Tags.

I think in theory it even would be possible to introduce a dependency
hierarchy for value types with a single root class.

Or perhaps there might be other way to avoid the direct exposore of
Nepomuk::Tag in tags() and tagsChanged(). At least tagsChanged() could
just emit a set of indices.

Regards,
Norbert