[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-core-devel
Subject: Re: Review Request: Pluggable KFileDialog/KAbstractFileModule
From: "Sebastian Trueg" <trueg () kde ! org>
Date: 2009-08-31 7:20:33
Message-ID: 20090831072033.6241.48267 () localhost
[Download RAW message or body]
> On 2009-08-30 22:27:01, David Faure wrote:
> > 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"...
The Nepomuk file dialog actually loads the traditional file module and allows to \
switch to it via button click. We initially tried to patch it into kdelibs but it has \
so many dependencies on things in playground (the nepomuk file dialog uses almost all \
existing Nepomuk features) that we dropped this approach and I thought of this one. \
It makes testing and developing the nepomuk file dialog very simple.
Thanks for reviewing. Will commit.
- Sebastian
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/1407/#review2202
-----------------------------------------------------------
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
>
>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic