[Differential] [Changed Subscribers] D2972: Allow configuring script's screen edges from the KCM

[graesslin (Martin Gräßlin)]

graesslin added inline comments.

INLINE COMMENTS

> main.cpp:95-96
>  
> +
> +
>      monitorLoad();

nitpick

> main.cpp:212
>  
> +    KService::List offers = KServiceTypeTrader::self()->query("KWin/Script", "[X-KWin-Border-Activate] == true and not (exist [X-KWin-Exclude-Listing]) or [X-KWin-Exclude-Listing] == false");
> +    QList<KPluginInfo> scripts = KPluginInfo::fromServices(offers);

Why are you querying KService? Scripts are packages and at least KWin internally they are queried using:

  KPackage::PackageLoader::self()->listPackages(QStringLiteral("KWin/Script"), scriptFolder);

> main.cpp:216
> +    KConfigGroup config(m_config, "Plugins");
> +    foreach (const KPluginInfo &script, scripts) {
> +        if (!config.readEntry(script.pluginName() + QStringLiteral("Enabled"), script.isPluginEnabledByDefault())) {

please don't add any new foreach any more. It might get deprecated

> main.cpp:328
> +        list = scriptConfig.readEntry("BorderActivate", list);
> +        foreach (int i, list) {
> +            monitorChangeEdge(ElectricBorder(i), index);

same here

REPOSITORY
  rKWIN KWin

REVISION DETAIL
  https://phabricator.kde.org/D2972

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: davidedmundson, #plasma
Cc: graesslin, plasma-devel, kwin, lesliezhai, ali-mohamed, hardening, jensreuterberg, abetts, sebas
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20161007/8cff3af5/attachment.html>