D26398: [KCM/Activities] Use KConfigXT in ui

Méven Car noreply at phabricator.kde.org
Wed Jan 8 11:16:40 GMT 2020


meven added inline comments.

INLINE COMMENTS

> ervin wrote in BlacklistedApplicationsModel.cpp:179
> I personally like that construct, but AFAIK it's rather unusual in KDE code, so maybe for the sake of the future developer use something more "classic".
> Either:
> 
>   const auto name = d->applications[i].name;
>   if (d->applications[i].blocked) {
>       blockedApplications << name;
>   } else {
>       allowedApplications << name;
>   }
> 
> Or:
> 
>   auto &list = (d->applications[i].blocked ? blockedApplications : allowedApplications);
>   list << d->applications[i].name;
> 
> Second option is closer to your initial intent (I think it's still less common than the first, but at least we guide the future reader understanding with the intermediate variable).

It was just some copy/paste will update to the second option i tink

> ervin wrote in BlacklistedApplicationsModel.cpp:186
> It shall work as intended for now, but I think there's a potential caveat there in case of future refactoring and if the timing between save()/load() of the various settings object change. The isSaveNeeded()/isDefaults() state will be based on the whole settings object state. So we might claim here for saving being needed (currently unlikely AFAICT) or settings not being defaults (currently likely if load() brought values from the file which weren't defaults) based on config keys which are not managed by this class. I thus wonder if it wouldn't be wise to restrict that to only the items concerned.

I tride this, but I can't access here to the default value of  blockedApplications and allowedApplications as the generated function `defaultAllowedApplicationsValue_helper()` is protected.
I was missing an alternative.

REPOSITORY
  R119 Plasma Desktop

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

To: meven, #plasma, ervin, bport
Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20200108/3aa06bd3/attachment-0001.html>


More information about the Plasma-devel mailing list