> On July 25, 2013, 4:33 p.m., David Faure wrote:
> > staging/kservice/src/plugin/kpluginloader.cpp, line 235
> > <http://git.reviewboard.kde.org/r/111686/diff/1/?file=173289#file173289line235>
> >
> > This relates to my comment in the other review. If what we have is a
> > list with always just one parameter (which is a map), why not pass the map
> > directly in setArgs, rather than a list of maps with one map in the list?
> >
> > IOW: setArgs(QVariantMap).
>
> Sebastian Kügler wrote:
> For this to work, we also need the macro to include the json file. That
> means that we have to introduce a new DECLARATION (temp-)macro to allow
> passing in a .json file. I was planning to put this ahead, and get this
> merged first, but as we need a coherent solution for the whole chain, a new
> version of the patch will add this macro as well, and then we can synchronize
> it also with the KPluginInfo ctor patch. I'm working on these changes right
> now, then I'll look into how the layout of the arguments passed around should
> look like.
This is what we get from QPluginLoader::metaData() when baking in a simple json
file:
QJsonObject({"IID": "org.kde.KPluginFactory","MetaData": {"Comment": "Date and
time by timezone","Icon": "preferences-system-time","Name": "Date and
Time","Type": "Service","X-KDE-Library":
"plasma_engine_time","X-KDE-PluginInfo-Author": "Aaron
Seigo","X-KDE-PluginInfo-Category": "Date and Time","X-KDE-PluginInfo-Depends":
[],"X-KDE-PluginInfo-Email":
"ase...@kde.org","X-KDE-PluginInfo-EnabledByDefault":
true,"X-KDE-PluginInfo-License": "LGPL","X-KDE-PluginInfo-Name":
"time","X-KDE-PluginInfo-Version": "1.0","X-KDE-PluginInfo-Website":
"http://plasma.kde.org/","X-KDE-ServiceTypes":
["Plasma/DataEngine"]},"className": "factory","debug": false,"version": 328192})
The ctor takes a QVariantList.
>From the other review:
> Not exactly, it returns a QJsonObject, which has a toVariantMap().
> Correct me if I'm wrong (I don't know anything about JSON), but I don't see
> where the "list of maps" comes from.
It comes from the type of the arg passed into the plugin. We want to keep this
compatible, changing the QVariantList args to a map would mean porting all the
plugins, i.e. nope.
This question then very much boils down to where to do the conversion from List
to Map, i.e. where are we putting the metadata, assuming we want to keep all
the metadata, and not just certain keys (which we'd still need to somehow put
in a list). I've chosen to do the conversion (i.e. the args <<
QPluginLoader::metaData().toVariantMap() in the factory method, since that
keeps the ctor, args() and setArgs() of KPluginLoader synchronous (i.e. no
mixture of Map and List there).
As to the question "What happens if there's more than one "MetaData" map in
there. We can take either the first, or the last one, taking the first seems
sensible, since we know that's where Qt puts it.
- Sebastian
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/111686/#review36497
-----------------------------------------------------------
On July 25, 2013, 4:10 p.m., Sebastian Kügler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/111686/
> -----------------------------------------------------------
>
> (Updated July 25, 2013, 4:10 p.m.)
>
>
> Review request for KDE Frameworks and David Faure.
>
>
> Description
> -------
>
> Make K_EXPORT_PLUGIN work with Qt's new plugin system
>
> This patch changes the K_EXPORT_MACRO and the class it generates to be
> compatible with Qt's new plugin / metadata system. It basically replaces the
> old macros around q_plugin_instance with the new ones, using Q_INTERFACES.
> There's also a setter for the args, which are used to pass metadata into the
> plugin.
>
> Otherwise, this is the minimal change, to make old plugin factories work atop
> the new framework.
>
> This change is source-compatible, but the right .moc file when this macro is
> used from the .cpp file.
>
>
> Diffs
> -----
>
> staging/kservice/src/plugin/kexportplugin.h cc5d58b
> staging/kservice/src/plugin/kpluginfactory.h a5ea21b
> staging/kservice/src/plugin/kpluginfactory.cpp 6bd2350
> staging/kservice/src/plugin/kpluginfactory_p.h 09fcfe4
> staging/kservice/src/plugin/kpluginloader.cpp 945c75b
>
> Diff: http://git.reviewboard.kde.org/r/111686/diff/
>
>
> Testing
> -------
>
> Loaded plugins using KService, KPluginLoader, QPluginLoader and
> Plasma::PluginLoader, all work as expected.
>
>
> Thanks,
>
> Sebastian Kügler
>
>
_______________________________________________
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
