This revision was automatically updated to reflect the committed changes.
Closed by commit R237:be28e096c533: Add force save behavior to KEntryMap
(authored by bport).
REPOSITORY
R237 KConfig
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D28128?vs=78861&id=80383
REVISION DETAIL
ht
ervin accepted this revision.
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
dfaure accepted this revision.
This revision is now accepted and ready to land.
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
bport updated this revision to Diff 78861.
bport marked 3 inline comments as done.
bport added a comment.
Take in consideration ervin feedback
REPOSITORY
R237 KConfig
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D28128?vs=78503&id=78861
REVISION DETAIL
https://phabricator.kde.
ervin added inline comments.
INLINE COMMENTS
> kconfigtest.cpp:1953
> +
> +void KConfigTest::testKdeglobalsVSDefault()
> +{
Seeing how this test confuses everyone (including me and I knew the problem
before hand...), I think it'd benefit greatly from getting comments at the most
important poin
bport updated this revision to Diff 78503.
bport added a comment.
Take in consideration dfaure feedback.
REPOSITORY
R237 KConfig
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D28128?vs=78432&id=78503
REVISION DETAIL
https://phabricator.kde.org/D28128
AFFECTED FILES
autotests
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
bport updated this revision to Diff 78432.
bport added a comment.
- fix when entry is a string (entry=) lead to empty string not fallback to
default value
- can't find a way to use bReverted / bDeleted without bForceSave because
bReverted is set when we call revertEntry and at this time we
dfaure added inline comments.
INLINE COMMENTS
> bport wrote in kconfigtest.cpp:1970
> This is a global local file, not system wide and so not considered as default
> cf. https://lxr.kde.org/source/frameworks/kconfig/src/core/kconfig.cpp#0702
> if the entry is set system wide
> /etc/kde5rc
> /etc
bport planned changes to this revision.
bport added inline comments.
INLINE COMMENTS
> dfaure wrote in kconfigtest.cpp:1965
> This would pass no matter in which file the write happened, no, due to
> caching?
> Doesn't this need a generalLocal.reparseConfiguration() to be meaningful?
yes indeed
dfaure requested changes to this revision.
dfaure added a comment.
This revision now requires changes to proceed.
Sounds like you need another feature, not breaking an existing one.
INLINE COMMENTS
> kconfigtest.cpp:1965
> +QVERIFY(local.sync());
> +QCOMPARE(generalLocal.readEntry("te
bport added a dependent revision: D28221: Don't write default value to
configuration file when default value came from /etc/* file.
REPOSITORY
R237 KConfig
REVISION DETAIL
https://phabricator.kde.org/D28128
To: bport, ervin, dfaure, meven, crossi, hchain
Cc: kde-frameworks-devel, LeGast00n,
bport marked 2 inline comments as done.
bport added a comment.
Added a unittest
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
bport updated this revision to Diff 78289.
bport added a comment.
Add unit test
REPOSITORY
R237 KConfig
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D28128?vs=77925&id=78289
REVISION DETAIL
https://phabricator.kde.org/D28128
AFFECTED FILES
autotests/kconfigtest.cpp
autote
dfaure requested changes to this revision.
dfaure added a comment.
This revision now requires changes to proceed.
I would prefer a unittest based on public API, showing the actual bug (before
the fix) with kdeglobals and a local config file.
INLINE COMMENTS
> kconfigdata.h:76
> +/**
> +
meven accepted this revision.
This revision is now accepted and ready to land.
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
meven added inline comments.
INLINE COMMENTS
> kconfigdata.cpp:139
>
> +// If overidded entry is global and not default. And it's overrided by a
> non global
> +if (e.bGlobal && !(options & EntryGlobal) && !k.bDefault)
s/overidded/overridden
REPOSITORY
R237 KConfig
REVISION DETAIL
bport created this revision.
bport added reviewers: ervin, dfaure, meven, crossi, hchain.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
bport requested review of this revision.
REVISION SUMMARY
Fix the following bug, if an entry is set on HOME/.config/kdegl
18 matches
Mail list logo