-----------------------------------------------------------
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
> 
>

Reply via email to