Overhaul KPluginSelector
Open, Needs TriagePublic

Description

The usage of KService/KPluginInfo should be replaced with KPluginLoader/KPluginMetaData.

Furthermore, a QML version would be good to have

ognarb claimed this task.Sep 29 2020, 2:16 PM
nicolasfella moved this task from Backlog to In Progress on the KF6 board.Nov 22 2020, 12:12 AM
alex added a subscriber: alex.Jul 2 2021, 5:42 AM

To what extend do we need to keep the old plugin querying around in the replacement?

I am thinking that in case of applications where all the plugins are shipped with the app itself we could safely drop this, but in the KRunner KCM we might want to support that because of third party runners which ship their own config. Maybe we could add a property to signal the KPluginSelector replacement that the old kservicetypetrader paths should be used?

I have a WIP at https://invent.kde.org/nicolasfella/kcmutils/-/commit/839f9337eb954fda99d369c4fe5d96395b967748

The key differences to KPluginSelector are:

  • A separate mode, to be used for a QML variant
  • Based on KPluginMetaData instead of KPluginInfo
  • plugin KCMs are determined by the X-KDE-ConfigModule

It still needs some work/real word testing to iron out the API/functionality. Also it still misses some of the new-ish KPluginSelector features like default indicators

alex added a comment.EditedAug 2 2021, 7:00 PM

Thanks for the preview, I think it goes in a really good direction.

plugin KCMs are determined by the X-KDE-ConfigModule

I am not sure about this. Shouldn't ideally be all KCMs be in a namespace specific to the app? Like kf5/krunner/kcms.

Otherwise one will have the issue that querying the metadata will be slow and we will have a monolithic dir full with all kind of KCMs.

How about having to provide a namespace when constructing the object and plugins could overwrite it with the X-KDE-ConfigModuleNamespace property that was discussed in https://phabricator.kde.org/T13555#259170. Then the case where the baloo runner wants to load the baloo system settings KCM is handled too.

Of course forcing consumers to provide the full path would also be an option, but that is rather ugly.

davidre added a subscriber: davidre.Aug 3 2021, 9:14 AM
In T12265#261244, @alex wrote:

Thanks for the preview, I think it goes in a really good direction.

plugin KCMs are determined by the X-KDE-ConfigModule

I am not sure about this. Shouldn't ideally be all KCMs be in a namespace specific to the app? Like kf5/krunner/kcms.

Why not both? You still need to map from a plugin to it's config module but we can install them into a specific dir next to the plugins or in the same folder

I think we have some misunderstanding here.

For example in KDE Connect we have n plugins that are installed into <plugins>/kdeconnect. Some of those plugins have a settings module. The plugin KCMs are currently in the plugin root dir, but we might as well install them into <plugins>/kdeconnect/pluginconfig. That would not solve anything on its own though since we still need a way to map from a plugin to its KCM, e.g. kdeconnect_pausemusic -> kdeconnect_pausemusic_config. In theory we could infer that purely on the name, but I'm not sure we want to, given that we currently don't enforce any naming scheme and then we wouldn't have a way to detect whether a plugin has a KCM at all without trying to load one.

alex added a comment.Aug 6 2021, 6:55 AM

That would not solve anything on its own though since we still need a way to map from a plugin to its KCM

That is correct, however plugins are usually installed into a specific namespace. This is related to the issue that for plasma applets the plugins in the root dir are queried.
It would also make things consistent with the existing usages of namespaces, for example kwin uses kwin/effects/configs/specifically for their KCMs.

With your kdeconnect example all the kdeconnect specific plugins would be in the kdeconnect dir (and the subdirs), which is IMHO very nice.

As said in my comment above consumers could write the full path to the KCM, including the namespace. Like kdeconnects/kcms/somekcm or kdeconnects/configs/somekcm, but that will lead to quite a few unnecessary duplications of the namespace and it could easily be overlooked by API consumers.

Also please use the KPluginMetaData overload to create the KCModuleProcxy instance.

alex added a comment.Sep 14 2021, 7:16 AM

As discussed in the weekly providers should use a specific namespace, but that should not get enforced in the API

ognarb removed ognarb as the assignee of this task.Oct 19 2021, 3:35 PM
ognarb added a subscriber: ognarb.
alex claimed this task.Nov 16 2021, 4:26 PM
alex added a comment.Nov 18 2021, 8:34 AM

Thanks, I already have made a plasma-desktop MR, which is similar, but utilizes the missing features of KPluginWidget I have added

My frameworks MR is:
https://invent.kde.org/frameworks/kcmutils/-/merge_requests/68

And some porting MRs:
https://invent.kde.org/plasma/kwin/-/merge_requests/1680
https://invent.kde.org/plasma/plasma-desktop/-/merge_requests/701

I fixed all the issues with the state changes & the plugin loading. I also made sure the change does not depend on any deprecated KCMUtils API.

I am planning on doing some minor changes to the signals of the KPluginWidget, for example there is no easy way to forward the changed state to the KCModule::changed(bool) signal.

KDevelop turns out somewhat tricky to port due to it's custom (and broken) handling of the defaults button: https://invent.kde.org/kdevelop/kdevelop/-/blob/master/kdevplatform/shell/settings/pluginpreferences.cpp#L75

Instead of calling defaults on the KPluginSelector it creates a new config to replace the old one and reloads the KPluginSelector. This is because if the plugin does not specify whether it is enabled by default KDevelop assumes it to be enabled but KPluginMetaData::isEnabledByDefault does not.

Cleaning that up by porting all KDevelop plugins to explicitly say they are enabled by default and porting KDevelop to the standard KPluginSelector/KPluginWidget defaults would fix some bugs and drop quite a bit of code, but I'm not sure about potential side effects

alex moved this task from In Progress to Done on the KF6 board.Dec 28 2021, 6:38 PM