D27934: KCM/Kwinoptions: Port focus tab to KConfigXT

Kevin Ottens <[email protected]>
Newsgroups gmane.comp.kde.devel.kwin
Message-ID <[email protected]>
ervin requested changes to this revision.
ervin added a comment.
This revision now requires changes to proceed.


  Not 100% about some of the comments, so let me know how this goes.

INLINE COMMENTS

> main.cpp:45
>      KFocusConfigStandalone(QWidget* parent, const QVariantList &)
> -        : KFocusConfig(true, new KConfig("kwinrc"), parent)
> +        : KFocusConfig(true, new KWinOptionsSettings(parent), parent)
>      {}

Is the KWinOptionsSettings object lifetime really tied to the parent of the KFocusConfigStandalone object? I would expect it to be tired to the lifetime of the KFocusConfigStandalone object itself.

> main.cpp:54
>      KMovingConfigStandalone(QWidget* parent, const QVariantList &)
> -        : KMovingConfig(true, parent)
> +        : KMovingConfig(true, new KWinOptionsSettings(parent), parent)
>      {}

ditto

> main.cpp:63
>      KAdvancedConfigStandalone(QWidget* parent, const QVariantList &)
> -        : KAdvancedConfig(true, parent)
> +        : KAdvancedConfig(true, new KWinOptionsSettings(parent), parent)
>      {}

ditto

> main.cpp:70
>  {
> -    mConfig = new KConfig("kwinrc");
> +    mSettings = new KWinOptionsSettings(parent);
>  

ditto

> main.cpp:84
> +    // Need to relay unmanagedWidgetDefaultState and unmanagedWidgetChangeState to wrapping KCModule
> +    connect(mFocus, &KFocusConfig::unmanagedWidgetDefaulted, this, [this](bool defaulted){
> +        unmanagedWidgetDefaultState(defaulted);

Is the lambda really necessary? I'd have expected a direct connect from unmanagedWidgetDefaulted to unmanagedWidgetDefaultState.

> main.cpp:87
> +    });
> +    connect(mFocus, &KFocusConfig::unmanagedWidgetStateChanged, this, [this](bool changed){
> +        unmanagedWidgetChangeState(changed);

ditto

> main.cpp:184
>  {
> -    mConfig = new KConfig("kwinrc");
> +    mSettings = new KWinOptionsSettings(parent);
>  

Is the lifetime really tied to parent and not this?

REPOSITORY
  R108 KWin

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

To: meven, ervin, bport, hchain, crossi, #kwin
Cc: 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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.