D28221: Don't write default value to configuration file when default value came from /etc/* file

[David Faure]

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20200324/1ec23a72/attachment.html>