D27682: Port kwinrules kcm to kconfigxt

ervin requested changes to this revision.
ervin added a comment.
This revision now requires changes to proceed.


  Minor things now (the one about the enum can be ignored of course).

INLINE COMMENTS

> ervin wrote in ruleslist.cpp:32
> I don't think this is doing what you expect, you didn't declare m_settings as a pointer. I wonder if that couldn't even lead to a double delete... might not be the case a bit by chance (since children are cleaned-up in QObject dtor the KCMRulesList part of the object (including the m_settings member are long gone). Cheer luck ;-)

I still think this should be switched to using a pointer, or if not the this should be dropped as parameter.

> rules.h:104
>      Rules();
> -    explicit Rules(const KConfigGroup&);
> +    explicit Rules(const RuleSettings*);
>      Rules(const QString&, bool temporary);

Can drop the const now

> rules.h:135
> +        UnusedSetRule = Unused,
> +        SetRuleDummy = 256   // so that it's at least short int
> +    };

Might be worth testing without it, since C++11 this should be guaranteed to be an int anyway.

REPOSITORY
  R108 KWin

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

To: hchain, meven, crossi, bport, ervin, #kwin
Cc: iasensio, ognarb, zzag, kwin, Orage, cacarry, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, mkulinski, ragreen, jackyalcine, iodelay, crozbo, bwowk, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, hardening, romangg, jensreuterberg, abetts, sebas, apol, ahiemstra, mart