KCM Style : kcm state is handled by ManagedConfigModule, properties are bound to settings, making it a bit simpler.
Needs ReviewPublic

Authored by crossi on Fri, Nov 8, 4:49 PM.

Details

Reviewers
ervin
bport
mart
broulik
Group Reviewers
Plasma

Diff Detail

Repository
R119 Plasma Desktop
Lint
Lint Skipped
Unit
Unit Tests Skipped
Build Status
Buildable 18715
Build 18733: arc lint + arc unit
crossi created this revision.Fri, Nov 8, 4:49 PM
Restricted Application added a project: Plasma. · View Herald TranscriptFri, Nov 8, 4:49 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
crossi requested review of this revision.Fri, Nov 8, 4:49 PM
crossi updated this revision to Diff 69467.Fri, Nov 8, 4:59 PM

make access to selectedStyle more consistent

crossi updated this revision to Diff 69468.Fri, Nov 8, 5:02 PM

missed that one

ervin added inline comments.Tue, Nov 12, 10:54 AM
kcms/style/kcmstyle.cpp
98–102

Space before &, not after.

245

AFAICT this is the only place where m_effectsDirty is read. So I think you can remove that flag altogether, which should bring further simplifications.

247–248

Are you sure you need this m_previousStyle? I'd expect it to be exactly m_settings->widgetStyle().

crossi added inline comments.Tue, Nov 12, 1:08 PM
kcms/style/kcmstyle.cpp
98–102

Will fix

245

m_effectsDirty is checked line 291, to trigger or not a refresh of all applications.

This check before save is now useless, since if we can save, it means a modification has occured, style or effects.

247–248

m_model->selectedStyle() is bound to m_settings->widgetStyle() and vice versa, so I need to save previous state to allow a rollback.
I should have used m_settings->widgetStyle() here and after for consistency.

ervin added inline comments.Tue, Nov 12, 1:45 PM
kcms/style/kcmstyle.cpp
247–248

As far as rollback goes (might not be enough in your case), wouldn't checking for isSaveNeeded() on the settings and rollbacking it all be enough?

We can't rollback a single item though, I guess that would be the main limitation in your case... Maybe that's something to add to KConfigSkeletonItem?

Answer might be no to both, I'm just looking for opportunities to simplify code in modules. :-)

crossi updated this revision to Diff 69635.Tue, Nov 12, 2:02 PM

style + consistency