Review Request: Pluggable KFileDialog/KAbstractFileModule

[David Faure]

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

Ship it!


Heh. This is funny side effect of the kde-4.0 change were we made kfile dlopened. The idea was not at all
to allow other implementations of it (rather it was just to delay the loading of that code). But ok, why not :)

About using the trader: indeed, not much point when the library name is already known (from the config).
Using the trader would make sense if you wanted the user to be able to configure the preferred "KIO/FileModule"
without knowing the name of the library (but simply by selecting in the list of available ones), or if you
wanted the mere installation of the alternate module to mean it should be used (InitialPreference field).
But indeed we don't want that in the case of the file dialog.

The fallback to loadFileModule("kfilemodule") could be done only if moduleName != "kfilemodule", btw,
to avoid trying the same thing twice.

Anyway. No real objections to the patch, but I wonder if the integration of nepomuk features in the filedialog
shouldn't be done with an additional "mode" in the existing filedialog rather than a hard all-or-nothing config item.
A user might want to sometimes "query nepomuk" and sometimes "use the old-fashioned way"...

- David


On 2009-08-28 13:16:59, Sebastian Trueg wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/1407/
> -----------------------------------------------------------
> 
> (Updated 2009-08-28 13:16:59)
> 
> 
> Review request for kdelibs.
> 
> 
> Summary
> -------
> 
> KIO already loads the widget used in the KFileDialog dynamically from libkfilemodule. The base class is KAbstractFileModule. However, the loaded lib/plugin is fixed. This small patch changes the situation making it really pluggable by using the standard KService procedure of loading plugins. It allows to easily change the file dialog implementation by creating a plugin of type "KIO/FileModule" and adding its name to the "kfilemodulerc" config file.
> The reason for this is that in the GSoC Alessandro Sivieri developed a replacement for the file dialog based on Nepomuk. He tried to patch it into kdelibs but it was a mess due to all the dependencies. With this approach implementations can easily be switched.
> 
> Open issues:
> * the patch uses KService::serviceByDesktopName but also contains commented code using KServiceTypeTrader. I am not sure which is preferable.
> * I left in the legacy code of loading the filemodule from libkfilemodule. I suppose this should be removed.
> 
> 
> Diffs
> -----
> 
>   trunk/KDE/kdelibs/kfile/CMakeLists.txt 1013393 
>   trunk/KDE/kdelibs/kfile/kfilemodule.desktop PRE-CREATION 
>   trunk/KDE/kdelibs/kfile/kfilemodule.h 1013393 
>   trunk/KDE/kdelibs/kfile/kfilemodule.cpp 1013393 
>   trunk/KDE/kdelibs/kio/CMakeLists.txt 1013393 
>   trunk/KDE/kdelibs/kio/kfile/kfiledialog.cpp 1013393 
>   trunk/KDE/kdelibs/kio/kfile/kiofilemodule.desktop PRE-CREATION 
> 
> Diff: http://reviewboard.kde.org/r/1407/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sebastian
> 
>