KConfig borkage
Oswald Buddenhagen
ossi at kde.org
Mon Oct 29 16:24:39 GMT 2007
On Sun, Oct 28, 2007 at 09:57:30PM -0500, Thomas Braxton wrote:
> This patch fixes the wierdness and
>
good
> removes entries from other locales from the entryMap,
>
good, but ...
> since I was already in the area ;)
>
cumulative patches suck. don't do them unless necessary (yes, svn makes
them necessary more often than one'd like it to, but not in this case).
> I also added a test, since the test for "stringEntry1[fr]" was wrong
> since KConfigGroup was saving the key as "stringEntry1\x5cfr\x5d", I
> think those are the right codes.
>
yes. you should have tracked the kconfig_new_take2 branch. ;)
pasting the patch wasn't particularly clever ... the line breaks don't
exactly help applying or reading it. :}
btw, gmail's mime type detection works for me - maybe the windows
lineendings were at fault or maybe gmail uses a mime type delivered by
the browser - dunno.
> Index: tests/kconfigtest.cpp
> ===================================================================
> --- tests/kconfigtest.cpp (revision 730047)
> +++ tests/kconfigtest.cpp (working copy)
> @@ -500,6 +527,23 @@
> + // test for entries that are marked as deleted when there is no default
> + KConfig config("kconfigtest", KConfig::SimpleConfig); // make sure there
> are no defaults
> + cg = config.group("Portable Devices");
> + cg.writeEntry("devices|manual|(null)", "whatever");
> + cg.writeEntry("devices|manual|/mnt/ipod", "/mnt/ipod");
> + cg.sync();
> +
> + int count=0;
> + foreach(const QByteArray& item, readLines())
> + if (item.startsWith("devices|"))
> + count++;
> + QVERIFY(count == 2);
> + cg.deleteEntry("devices|manual|/mnt/ipod");
> + cg.sync();
> + foreach(const QByteArray& item, readLines())
> + QVERIFY(item != "devices|manual|/mnt/ipod[$d]");
>
that doesn't test that the entry is gone, though.
i guess copying the startsWith("devices|") counter and comparing with
one would make more sense.
> Index: config/kconfigini.cpp
> ===================================================================
> --- config/kconfigini.cpp (revision 730047)
> +++ config/kconfigini.cpp (working copy)
> @@ -202,10 +202,8 @@
> if (!locale.isEmpty()) {
> if (locale != currentLocale) {
> // backward compatibility. C == en_US
> - if (locale.at(0) != 'C' || currentLocale != "en_US") {
> - aKey += '[' + locale + ']';
> - locale = QByteArray();
> - }
>
wow, this was impressively broken - locale appended after the key was
decoded.
> + // FIXME is this needed anymore?
>
why not?
> if ( !file.size() && ((fileMode == -1) || (fileMode == 0600)) ) {
> // File is empty and doesn't have special permissions: delete it.
> file.abort();
>
while trying to understand the rest of the patch i noticed that
EntryDefault isn't set anywhere. consequently all the logic building on
top of it is dysfuct. so there is little point in reviewing the patch,
i figure.
procedure:
- revert everything
- do the localized entry purge, commit
- add the failing tests (possibly come up with more?), commit
- fix cascaded parsing. revisit the problems. if they persist, post a
new patch.
--
Hi! I'm a .signature virus! Copy me into your ~/.signature, please!
--
Chaos, panic, and disorder - my work here is done.
More information about the kde-core-devel
mailing list