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

Reply via email to