KConfig borkage

[Thomas Braxton]

On 10/29/07, Oswald Buddenhagen <ossi at kde.org> wrote:
>
> 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.
>
this should now be fixed, update & let me know if anything doesn't work
right
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20071030/0a316657/attachment.htm>