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

[David Faure]

dfaure added a comment.


  Ah! Now it actually makes sense to me. If we are changing what revertToDefault() does, then it makes sense to change the if() condition for calling it. Basically, now that it does the right thing in both cases (default from C++ and default from system file) it can be called in both cases, while before it was only called if there was no default from a system file. Right?
  
  I'm still wondering about this though: "If we're saving a value equal to the C++ default, then we have to write it out, otherwise upon reading the global-file default will interfer."
  Your unittest shows that revertToDefault() will lead to the global-file value being read.
  Doesn't this mean that this code is wrong then?
  
     if (value == "cppdefault") {
        cg.revertToDefault(key);
    } else {
        cg.writeEntry(key, value);
    }
  
  That's the code you're using everywhere, so that's actually the code that would be good to unittest ;-)
  Isn't this buggy when the value actually *is* cppdefault, and there is a system config file with another default value?
  
  > Indeed probably need to update configuration too.
  
  Did you mean documentation? I know this is all about configuration ;) but the bit that's missing is to update the documentation, and add a task to get rid of the hasDefault() everywhere before revertToDefault is called. Assuming I'm wrong above about there being a bug left :-)

INLINE COMMENTS

> kconfigtest.cpp:1991
> +    KConfigGroup generalLocal(&local, "General");
> +    // Check if we get global and not the default value from cpp (defaultcpp) when reading data from restorerc
> +    QCOMPARE(generalLocal.readEntry("testKeyDefault", "defaultcpp"), "configdefault");

Bug in the comment, the file isn't called restorerc.

> kconfigtest.cpp:1998
> +    local.reparseConfiguration();
> +    // We expect to get configdefault value and configdefault not wrote to local file (file will not exist)
> +    QCOMPARE(generalLocal.readEntry("testKeyDefault", "defaultcpp"), "configdefault");

s/wrote/written/

> kconfigtest.cpp:2002
> +
> +    // Write a custom value, we expect this value to be wrote to local file
> +    generalLocal.writeEntry("testKeyDefault", "custom");

s/wrote/written/

REPOSITORY
  R237 KConfig

REVISION DETAIL
  https://phabricator.kde.org/D28221

To: bport, ervin, dfaure, davidedmundson
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20200426/d6e91e50/attachment.html>