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