Review Request: Add KMetaDataWidget and KMetaDataConfigurationDialog to kdelibs/kfile

[Peter Penz]

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

(Updated 2009-10-27 08:06:19.565297)


Review request for kdelibs, Sebastian Trueg and David Faure.


Changes
-------

Thanks for the review comments. I've uploaded an updated patch with the following changes:

* Removed the tags-, comments- and rating-APIs. As Sebastian said it is only an arbitrary subset of what Nepomuk can provide. If the application really requires to change those properties manually, it can do it directly with Nepomuk anyhow. Nice: The HAVE_NEPOMUK hack in the header is now gone.

* Use setShownData()/shownData() instead of setHiddenData()/hiddenData() as Aurélien suggests.

* Added methods setCustomDataList()/customDataList(), so that it is possible for applications like Gwenview to add a list of custom meta data. The method has not been implemented yet, but I mainly ask to check whether the interface and naming is OK. I thought about the possibility to add custom widgets instead of QString values, but I rejected the idea (this cries for some problems).

In general I'm against splitting KMetaDataWidget into 2 classes (one without Nepomuk and one with Nepomuk code). IMO the applications should not be burdened to instantiate the "correct" class dependent on whether Nepomuk is running or not. Whether Nepomuk is running or not is an implementation detail and reflected in the number of shown meta data, but it is not something that should result in the need of application code like this:

if (Nepomuk::ResourceManager::instance()->init() != 0) {
    m_metaDataWidget = new KMetaDataWidget(this);
} else {
    m_metaDataWidget = new KNepomukMetaDataWidget(this);
}

just let the application do this:
    m_metaDataWidget = new KMetaDataWidget(this);
and let KMetaDataWidget itself to take care about whether there is Nepomuk available or not.

Open issue:

* There is still the cmake warning mentioned above. I tried to use KDE4_NEPOMUK_LIBS instead of NEPOMUK_LIBRARIES after a hint from David, but in this case I even get a link error. Any help would be appreciated.


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 (updated)
-----

  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