Review Request 123669: Add KPluginLoader::findPluginsById convenience API

[Sebastian Kügler]

> 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
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20150606/68776693/attachment-0001.html>