dfaure requested changes to this revision.
dfaure added a comment.
This revision now requires changes to proceed.
Note that the docu for KConfigGroup::hasDefault has this logic too:
if ( (value == computedDefault) && !group.hasDefault(key) )
group.revertToDefault(key);
else
group.writeEntry(key, value);
With the reasoning that we want to avoid writing out the value if the value
is equal to computedDefault, UNLESS there's a system file that says otherwise.
Your change seems to break this.
I see the overall setup as this, ordered by priority:
[C++ computed default] < [system config files] < [user kdeglobals] < [user
app-specific config file]
!hasDefault() checks [system config files] and therefore should stay.
Otherwise, when the situation is C++=1 system=2 and value to be written is 1
you'll revert() i.e. not write anything and then re-read 2, oops.
Sounds like you should add a unittest for this case, to detect this
regression...
INLINE COMMENTS
> kconfigskeletontest.cpp:212
> + QVERIFY(glob.sync());
> + glob.reparseConfiguration();
> + auto s2 = new KConfigSkeleton(QStringLiteral("kconfigskeletontestrc"));
Is this needed? Nothing uses glob after this point.
> kconfigdata.cpp:319
> //qDebug() << "looking up default entry with key=" << defaultKey;
> const ConstIterator defaultEntry = constFind(defaultKey);
> + entry->mValue = QByteArray();
This means defaultEntry is now unused, and therefore defaultKey too.
REPOSITORY
R237 KConfig
REVISION DETAIL
https://phabricator.kde.org/D28221
To: bport, ervin, dfaure, davidedmundson
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns
