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
