> On 2010-07-10 08:00:51, Aaron Seigo wrote:
> > trunk/KDE/kdelibs/plasma/pluginloader.h, lines 31-41
> > <http://reviewboard.kde.org/r/4554/diff/1/?file=30585#file30585line31>
> >
> > a few issues here: the name of the file and the name of the class don't
> > match. it's also missing the PLASMA_EXPORT macro which is why you can't see
> > it outside of liblasma. but i don't think we need PluginLoaderManager in
> > any case.
> >
> > setPluginLoader and pluginLoader could just as easily be static methods
> > in ExternalPluginLoader with just one static pointer in
> > ExternalPluginLoader::Private.
> >
> > the only advantage i can think of for having a full-blown manager is to
> > offer an easy way eventually to layer multiple loaders. i'm not sure that's
> > even a feature we want, and if we do end up wanting it we can add that
> > fairly easily without a manager as well (or add a manager then and change
> > the implements of ExternalPluginLoader::setPluginLoader() to use that
> > internally)
Seems reasonable... I assume that I have to use K_GLOBAL_STATIC on the
ExternalPluginLoader class then, to make the static members available? The
build is currently failing:
[ 84%] Building CXX object plasma/CMakeFiles/plasma.dir/externalpluginloader.o
/home/rrix/dev/trunk/kdelibs/plasma/externalpluginloader.cpp:34: error:
‘externalPluginLoaderSingleton’ has not been declared
/home/rrix/dev/trunk/kdelibs/plasma/externalpluginloader.cpp:35: error:
expected constructor, destructor, or type conversion before ‘}’ token
make[2]: *** [plasma/CMakeFiles/plasma.dir/externalpluginloader.o] Error 1
make[1]: *** [plasma/CMakeFiles/plasma.dir/all] Error 2
make: *** [all] Error 2
I'll upload the current diff.
- Ryan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/4554/#review6457
-----------------------------------------------------------
On 2010-07-09 10:11:58, Ryan Rix wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/4554/
> -----------------------------------------------------------
>
> (Updated 2010-07-09 10:11:58)
>
>
> Review request for Plasma, Aaron Seigo and Shaun Reich.
>
>
> Summary
> -------
>
> Adds ability for applications which embed plasma to specify a class which
> plasma will query for Applet, DataEngine, and Service creation, giving them
> the chance to create these objects internally.
>
> The foremost use of this is to give Plasma the chance to ask applications
> when restoring layouts from disk, for applets which were based on QWidgets,
> such as the case in various Plasma dashboards which may embed QWidgets as
> plasma::applets ....
>
>
> Diffs
> -----
>
> trunk/KDE/kdelibs/plasma/CMakeLists.txt 1146180
> trunk/KDE/kdelibs/plasma/applet.cpp 1146180
> trunk/KDE/kdelibs/plasma/dataenginemanager.cpp 1146180
> trunk/KDE/kdelibs/plasma/externalpluginloader.h PRE-CREATION
> trunk/KDE/kdelibs/plasma/pluginloader.h PRE-CREATION
> trunk/KDE/kdelibs/plasma/pluginloader.cpp PRE-CREATION
> trunk/KDE/kdelibs/plasma/service.cpp 1146180
>
> Diff: http://reviewboard.kde.org/r/4554/diff
>
>
> Testing
> -------
>
> Well, this thing is about 90% there right now, everything works, etc, afaict,
> but I can't get the instance of the PluginManager outside of the Plasma
> namespace, if someone could help that would be great. :) I can commit some
> support into trunk/playground/base/shells/kpart
>
>
> Thanks,
>
> Ryan
>
>
_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel
