Review Request: Prevent KConfigGroup::revertToDefault from marking the kconfig as dirty if there's nothing to do

Oswald Buddenhagen ossi at kde.org
Thu Nov 17 22:28:33 GMT 2011


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/103168/#review8284
-----------------------------------------------------------


the function definitely seems to be a misnomer; according to your interpretation (which i consider correct wrt the api doc), revertToGlobal() would be a much more accurate name.

i'm not sure why the function actively copies the global value into the local value - that ensures that a changing global value will not be seen by the merged config, but i tend to think that this is exactly what you would *not* expect from this function. does it actually write out the reverted local value?

note that kconfigdata actually has a reverttodefault function, but it's not used (there was some barely convincing reason for not simply using it, but i couldn't tell now).

and of course i don't like your inefficient little hack. what did you expect? ;-P

when we are clear on the wanted semantics and you cannot figure out how to do it right i can help you, but i'll have to start almost from scratch, too.


- Oswald Buddenhagen


On Nov. 17, 2011, 3:41 p.m., David Faure wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/103168/
> -----------------------------------------------------------
> 
> (Updated Nov. 17, 2011, 3:41 p.m.)
> 
> 
> Review request for kdelibs, Thomas Braxton and Oswald Buddenhagen.
> 
> 
> Description
> -------
> 
> KConfig marks an entry as dirty even when calling revertToDefault (which doesn't write out anything if there is no entry in the local config file). In theory this could be fixed inside the big KEntryMap::setEntry method in kconfigdata.h, but that code is way too obscure and fragile for my eyes. This simpler fix seems to do the job, unless I'm missing something.
> 
> Once my review request for KConfig::isDirty is merged in, this could even be unit-tested... I can do that, but I would really like input on 1) a possible better/faster implementation, 2) the actual expected behavior of revertToDefault in all cases (value in global config file, no value in global config file.... hmm I guess in all cases all we want is no value in the local file, right?)
> 
> 
> Diffs
> -----
> 
>   kdecore/config/kconfiggroup.cpp 9e73eb7 
> 
> Diff: http://git.reviewboard.kde.org/r/103168/diff/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> David Faure
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20111117/662a1cfe/attachment.htm>


More information about the kde-core-devel mailing list