broulik added a comment.
Not a fan of the `AutoloadEnabledSavedRole` in the model
INLINE COMMENTS
> kcmkded.cpp:269
> + if (!idx.data(ModulesModel::ImmutableRole).toBool()) {
> + m_model->setData(idx, true, ModulesModel::AutoloadEnabledRole);
> }
I think instead you could have the model refuse to `setData` when immutable
> modulesmodel.cpp:128
> const bool autoloadEnabled = value.toBool();
> if (item.type == KDEDConfig::AutostartType
> + && item.autoloadEnabled != autoloadEnabled) {
Since generally `setData` only does things for `AutostartType`, perhaps move
that check out of the `switch`
> modulesmodel.cpp:235
> const bool autoloadEnabled = cg.readEntry("autoload", true);
> + const bool immutable = cg.isEntryImmutable("autoload");
>
Superfluous space
> modulesmodel.cpp:275
> endResetModel();
> + emit autoloadedModulesChanged();
> }
Check whether it actually changed before emitting it
REPOSITORY
R119 Plasma Desktop
REVISION DETAIL
https://phabricator.kde.org/D26784
To: bport, ervin, crossi, meven, broulik
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus,
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai,
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
