Add convenience for defaults/dirty states to KCoreConfigSkeleton
ClosedPublic

Authored by ervin on Oct 8 2019, 1:16 PM.

Details

Summary

It allows to verify if all the items of the skeleton are in their
default values or if they hold any value deviating from the latest
loaded values from KConfig.

We didn't really need this during the KCModule/QtWidgets time since we
could write KConfigDialogManager just fine without it. But for use with
QML and aiming at having similar magic in KQuickAddons::ConfigModule
such convenience functions will be needed.

Diff Detail

Repository
R237 KConfig
Lint
Lint Skipped
Unit
Unit Tests Skipped
Build Status
Buildable 17460
Build 17478: arc lint + arc unit
ervin created this revision.Oct 8 2019, 1:16 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptOct 8 2019, 1:16 PM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
ervin requested review of this revision.Oct 8 2019, 1:16 PM

In principle +++++

src/core/kcoreconfigskeleton.h
218

Is this an ABI break?

:/

ervin updated this revision to Diff 67507.Oct 8 2019, 2:18 PM

Second try, without breaking ABI... it's the best workaround I found in the current situation, I admit I died a bit inside.

Looks good to me

src/core/kcoreconfigskeleton.cpp
140

Do we need to make this

if (d->mIsDefaultImpl){
   return d->mIsDefaultImpl();
}
return false;

and initialize mIsDefaultImpl to nullptr

so that it doesn't crash if someone subclasses KConfigSkeletonItem directly and doesn't implement this?

kossebau added inline comments.
src/core/kcoreconfigskeleton.h
216

-> 5.64

ervin updated this revision to Diff 67522.Oct 8 2019, 8:51 PM

5.63 -> 5.64

ervin added inline comments.Oct 8 2019, 8:54 PM
src/core/kcoreconfigskeleton.cpp
140

Initializing to nullptr wouldn't help, you would get the same behavior than with the default constructor.

It means it doesn't crash but raises a std::bad_function_call exception, which I think is fine... it's the next best thing to a pure virtual, but it's caught at runtime. I don't think we can do better than this in the current situation.

src/core/kcoreconfigskeleton.h
218

*gasp* you're right, how stupid of me... we can't merge that...

ervin added a comment.Oct 10 2019, 5:44 AM

Any chance to get another round of reviews now that this patch changed quite a bit?

apol added a subscriber: apol.Oct 10 2019, 10:40 AM

Looks better, it could make sense to add a KF6 TODO/Warning.

ervin updated this revision to Diff 67597.Oct 10 2019, 11:26 AM

Add the KF6 comment

davidedmundson accepted this revision.Oct 11 2019, 9:24 AM
This revision is now accepted and ready to land.Oct 11 2019, 9:24 AM
apol accepted this revision.Oct 11 2019, 9:30 AM
This revision was automatically updated to reflect the committed changes.