KPluginFactory/-Loader as KLibFactory replacement and KDE enhanced QPluginLoader

[Matthias Kretz]

First: I'm very much in favour of getting this ready for next monday. At least 
the KPluginFactory part has to be done before 4.0. The KPluginLoader could be 
an addition after 4.0 but I'd rather put it in know and see how much of the 
KLibLoader API we can remove once we have KPluginLoader. And we could also 
remove kgenericfactory.h, kgenericfactory.tcc and ktypelist.h if we find a 
good way to automatically port away from KGenericFactory (though I don't see 
us having the man power to do this).

> Index: util/klibloader.h
[...]
> +    template <typename T>
> +    KDE_DEPRECATED
> +    static T *createInstance( const QString &libname, QObject *parent,
> +                              const QStringList &args,
> +                              int *error = 0 )
> +    {
>          KLibrary *library = KLibLoader::self()->library( libname );
>          if ( !library )
>          {
> @@ -416,7 +455,7 @@
>                  *error = ErrNoLibrary;
>              return 0;
>          }
> -        KLibFactory *factory = library->factory();
> +        KPluginFactory *factory = library->factory();
>          if ( !factory )
>          {
>              library->unload();
> @@ -424,7 +463,7 @@
>                  *error = ErrNoFactory;
>              return 0;
>          }
> -        QObject *object = factory->create( parent, 
T::staticMetaObject.className(), args );
> +        QObject *object = factory->create<T>(parent, QVariantList() << 
args);

careful with the QStringList -> QVariantList conversion. In KPluginFactory you 
have a different method for conversion. OTOH you could probably just call
factory->create<T>(parent, args) here, it'll just result in two deprecation 
warnings.

> Index: util/klibloader_p.h

If KLibFactory is removed this file is not necessary.

> --- util/kpluginfactory.cpp     (revision 0)
> +++ util/kpluginfactory.cpp     (revision 0)
[...]
> +KPluginFactory::KPluginFactory(const char *componentName, const char
> *catalogName, QObject *parent) 
> +    : QObject(parent), d_ptr(new KPluginFactoryPrivate)
> +{
> +    Q_D(KPluginFactory);
> +    d->componentData = *createComponentData();

This one is for compatiblity? For one it's a mem leak. Do it like this:
  KComponentData *kcd = createComponentData();
  if (kcd) {
      d->componentData = *kcd;
      delete kcd;
  }
OTOH I don't like to have two createComponentData methods. Can you tell me why 
we need the createComponentData(KAboutData *) function?

[...]
> +KPluginFactory::KPluginFactory(QObject *parent)
Where is this ctor needed? Not enough to have the "KPluginFactory(const char 
*componentName = 0, const char *catalogName = 0, QObject *parent = 0)" ctor?

[...]
> +KComponentData *KPluginFactory::createComponentData()
> +{
> +    return new KComponentData();
> +}

This one should return 0 now.

> Index: util/kpluginfactory.h
[...]
> +    template<class impl>
> +    static QObject *createInstance(QObject *parent, const QVariantList
> &args)
> +    { 
> +        return new impl(parent, args);
> +    }

Perhaps we'll need another one to ease porting:
  template<class impl>
  static QObject *createOldInstance(QObject *parent, const QVariantList &args)
  { 
      return new impl(parent, variantListToStringList(args));
  }

This could ship with the following macro:

  #define K_REGISTER_OLD_PLUGIN(impl, iface) \
registerPlugin(iface::staticMetaObject.className(), &createOldInstance<impl>, 
QString());

Then the current code looking like this:
typedef KGenericFactory<MyPluginImpl> MyFactory;
K_EXPORT_COMPONENT_FACTORY(libname, MyFactory("componentName"))

could automatically be ported to:
K_PLUGIN_FACTORY(MyFactory, K_REGISTER_OLD_PLUGIN(MyPluginImpl, QObject))
K_EXPORT_PLUGIN(MyFactory)
the interface part is a bit of a problem though, often what we want is 
MyPluginImpl::staticMetaObject().superClass()->className() instead of QObject

> +    virtual KComponentData createComponentData(const KAboutData
> *aboutData);
> + 
> +    virtual KDE_DEPRECATED KComponentData *createComponentData();

like I said above: I'd like to reduce those two functions to one

> +    virtual void setupTranslations();

What are the use cases for having this virtual? I know KGenericFactory had it 
virtual, but was it ever reimplemented?

> +    void initializeMessageCatalog();

Do we need that in the public class. I'd like to move this function to the 
private if possible.

> +    virtual KDE_DEPRECATED QObject *createObject(QObject *parent, const
> char *className, const QStringList &args);

AFAIK marking this function deprecated only gives a warning when compiling 
KPluginFactory, not when reimplementing the function.

[...]
> Index: util/kpluginloader.h
[...]
> +#include <QtCore/qplugin.h>
That's <QtCore/QtPlugin>

[...]
> +#define K_EXPORT_PLUGIN2(library, factory) \
> +Q_EXPORT_PLUGIN2(library, factory) \
> +K_PLUGIN_VERIFICATION_DATA
> +
> +#define K_EXPORT_PLUGIN(factory) K_EXPORT_PLUGIN2(factory, factory)

Do we need this?
First question: Will we ever support statically compiled in plugins? If not 
then 
#define K_EXPORT_PLUGIN(factory) \
Q_EXPORT_PLUGIN(factory) \
K_PLUGIN_VERIFICATION_DATA

is enough. Otherwise we should define
#define K_EXPORT_PLUGIN(library, factory) \
Q_EXPORT_PLUGIN2(library, factory) \
K_PLUGIN_VERIFICATION_DATA

For max convenience (and too many macros probably) we could also provide
K_EXPORT_PLUGIN_FACTORY(name, pluginRegistrations) \
K_PLUGIN_FACTORY(name, pluginRegistrations) \
K_EXPORT_PLUGIN(name)

> +class KDECORE_EXPORT KPluginLoader : public QPluginLoader
> +{
> +    Q_OBJECT
> +    Q_PROPERTY(QString fileName READ fileName)
> +    Q_PROPERTY(QString pluginName READ fileName)
> +public:
> +    explicit KPluginLoader(const QString &plugin, const KComponentData
> &componentdata = KGlobal::mainComponent(), QObject *parent = 0); +   
> explicit KPluginLoader(const KService &service, const KComponentData
> &componentdata = KGlobal::mainComponent(), QObject *parent = 0); +   
> ~KPluginLoader();
> +
> +    KPluginFactory *factory();
> +
> +    QString pluginName() const;
> +
> +    bool isLoaded() const;
> +
> +    bool load();

What is this method needed for if it's already called automatically from the 
ctor?

-- 
________________________________________________________
Matthias Kretz (Germany)                            <><
http://Vir.homelinux.org/
MatthiasKretz at gmx.net, kretz at kde.org,
Matthias.Kretz at urz.uni-heidelberg.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20070824/671c9a90/attachment.sig>