Details
Diff Detail
- Repository
- R119 Plasma Desktop
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
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(). |
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. |
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. :-) |
kcms/style/kcmstyle.cpp | ||
---|---|---|
245 | Not for this patch, but I still wonder if we could kill it. I think it'd be better to have it as a local variable whose value would be computed from the state of the relevant items in the settings object. This would remove the logic of maintaining an extra state exploited only in save(). Also "effectsDirty" is a weird name I think, it doesn't seem related to any concept of effects. |