D28128: Add force save behavior to KEntryMap

[David Faure]

dfaure added a comment.


  I did some debugging.
  
  e.bForceSave is set to true at the time of doing the writeEntry("testRestore", "restore") -- or if I split this and create another KConfig instance (I thought this would fail...) it is then set while loading the file (good).
  
  The naming of that bool sounds weird though, because it sounds like it might force saving in cases where we don't want that.
  
  But in fact this bool is only used when reverting. So it should be renamed to something like bRevertShouldSave or bSaveWhenReverting. It has no effect on "normal saving".
  Or maybe it should be called based on what we know at the time of parsing, rather than on what it's going to be used for. Something like bOverridesGlobal?
  
  Sorry to nitpick on naming, but I think the patch would make a lot more sense, and the next reader would be much less confused by all this.
  
  Also, what happens if both kdeglobals and the local file have the same value? Should there be a e.mValue != value check after `e = *it`? In fact, the new code could move there since this can only happen if we *found* an existing entry, no? The test passes for me with http://www.davidfaure.fr/2020/moved.diff

INLINE COMMENTS

> kconfigdata.h:170
>          EntryNotify = 128,
> +        EntryForceSave = 256,
>          EntryDefault = (SearchDefaults << 16),

This enum value isn't set anywhere except in the unittest, no?
I guess it's here for consistency, but it still seems pretty useless?

REPOSITORY
  R237 KConfig

REVISION DETAIL
  https://phabricator.kde.org/D28128

To: bport, ervin, dfaure, meven, crossi, hchain
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20200325/e0274719/attachment.html>