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

[David Faure]

> On Nov. 17, 2011, 10:28 p.m., Oswald Buddenhagen wrote:
> > 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.
> >

It seems I forgot to answer your question, so this stalled, but I'd still like to see it fixed, it's causing user-visible delays when closing kmail composer windows (due to lots of KConfig sync'ing for nothing).

Yes, revertToGlobal() would be a better name indeed, since "default" is the second arg of readEntry, I presume. I can deprecate revertToDefault in favor of a revertToGlobal in KF5, and port kdelibs to that, if you want.

I can't really answer the questions about the current code, I didn't write it :)
I would welcome your help in fixing this better indeed.
I don't see a KConfigData class, not a revertToDefault outside of KConfigGroup, so I wonder what you saw and where...


- David


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


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/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> David Faure
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20121128/02cf1bc3/attachment.htm>