> On May 7, 2015, 7:24 a.m., Alex Richardson wrote: > > Seems like this is duplicated in a few places already so I agree we should > > add it. But won't most users of the API want only a single plugin returned? > > Maybe also add a function `KPluginMetaData > > KPluginLoader::findPluginById(const QString& directory, const QString& > > pluginId)`? > > Do we also need the function that returns a vector for a given ID? > > Sebastian Kügler wrote: > At least our changes in libplasma need a QVector<KPluginMetaData>. > Otherwise, a list seems easy enough to check if something's found. Returning > a single metadata won't be very useful for us at this point (but I see it > making sense). > > Sebastian Kügler wrote: > Ow, also, and perhaps more importantly, multiple ids are technically > perfectly valid (only the plugin name and directory are important here). I > think that fact should be reflected in the API. Perhaps a word on ordering > would be in place in the API docs, plugin locations are cascaded properly in > code using it. The most local plugin is at the end of the list, system-widely > installed ones at the beginning, so code that uses plugins.first() would not > allow the user to override plugins installed for example into /usr/lib with a > plugin with the same id and relative path installed into ~/.local). So an > extra method returning the .last() plugin found might take away this caveat > from the API. We'll still need the method returning a vector for libplasma, > though (and I think it's a semantically useful addition). > > About adding another method to return the most-local plugin, I'm on the > edge. If others think it's useful and we think the additional API (with its > long-term maintainance implications) is worth it, I'm happy to add it as > well. (Perhaps in a separate review.) > > Opinions welcome. > > Alex Richardson wrote: > The problem is that .last() won't work either. > QCoreApplication::libraryPaths() has this order > (http://code.woboq.org/qt5/qtbase/src/corelib/kernel/qcoreapplication.cpp.html#_ZN16QCoreApplication12libraryPathsEv): > $QT_INSTALLDIR/plugins, then the current executable directory and then > QT_PLUGIN_PATH. This means that the lowest priority entry from QT_PLUGIN_PATH > will be chosen in that case.
So, can I ship it with the QVector<>? Patch has been lingering for a bit, would be nice to get it in (we have just written another potential user of this method). - Sebastian ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123669/#review80013 ----------------------------------------------------------- On May 6, 2015, 11:21 p.m., Sebastian Kügler wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/123669/ > ----------------------------------------------------------- > > (Updated May 6, 2015, 11:21 p.m.) > > > Review request for KDE Frameworks, Alex Richardson and David Faure. > > > Repository: kcoreaddons > > > Description > ------- > > Add findPluginsById convenience API > > It's a quite common case to load a plugin from an ID. This makes it > easy. > > CHANGELOG:New KPluginLoader::findPluginById() convenience API > REVIEW:123669 > > > Diffs > ----- > > autotests/kpluginloadertest.cpp 3ded0ebca2e0fd20e09bf6e4eca152d13ac11f46 > src/lib/plugin/kpluginloader.h 124bfab7e207b17d3c8ab4d5a88321af476aad42 > src/lib/plugin/kpluginloader.cpp 4310d6cd7792329511f12b28d7c68b0fdd013538 > > Diff: https://git.reviewboard.kde.org/r/123669/diff/ > > > Testing > ------- > > Added autotests, everything passes. > > > Thanks, > > Sebastian Kügler > >
_______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
