[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-core-devel
Subject:    Re: even more on kconfig escapes (Re: KDE/kdelibs/kdeui/icons)
From:       Oswald Buddenhagen <ossi () kde ! org>
Date:       2007-11-23 0:13:41
Message-ID: 20071123001341.GA32124 () ugly ! local
[Download RAW message or body]

On Thu, Nov 22, 2007 at 05:05:06PM -0600, Thomas Braxton wrote:
> On 11/22/07, Andreas Pakulat <apaku@gmx.de> wrote:
> > On 22.11.07 16:36:06, Thomas Braxton wrote:
> > > +        return mName.mid(mName.lastIndexOf("/")+1);
> > > +    }
> > > I think the last line of name() is wrong, mName only contains a slash
> > > if the group was created with a name that contained a slash (i.e. the
> > > directories of an icon theme). So this should probably be reverted and
> > > change KConfigGroup::name() to just return mName.
> >
yes, see the attached patch, but at this point it is no fix, just an
optimization.

> > > +            groups << groupname.left(groupname.indexOf("/"));
> > > this will probably cause problems somewhere, because it is  assuming
> > > that if there's a slash in the name it is a group separator.
> >
> > Which is correct as of now, because thats the separator that was chosen
> > for nested groups. The reason I changed this was that one couldn't
> > create groups by using groupList(), i.e.
>
> > KConfigGroup grp(config, "someparent");
> > foreach(QString subgrp, grp.groupList())
> > {
> >   KConfigGroup somegrp(grp, subgrp);
> > }
> >
> > Didn't work as expected, i.e. if the file had [someparent/foo] with
> > entries I didn't get that group, but instead got the group
> > [someparent/someparent/foo].
> >
> > Thats IMHO just plain wrong, I mean I'd expect the groupList() of
> > KConfig to do this maybe, but certainly not the groupList() on
> > KConfigGroup as that one can be nested.
> 
> then appending groupname, not groupname.left(...) is probably what you want.
> 
no, that code is correct. you want only direct children of a group.
e.g., if the hierarchy someparent/foo/blubb exists, you don't want
foo/blubb, only foo. that's correct even if foo itself does not contain
any entries.

-- 
Hi! I'm a .signature virus! Copy me into your ~/.signature, please!
--
Chaos, panic, and disorder - my work here is done.

["kconfig-name.diff" (text/x-diff)]

commit 9c57e599a7bc7cc623623f2026ce323d56b8bf4d
Author: Oswald Buddenhagen <ossi@ugly.local>
Date:   Fri Nov 23 00:53:04 2007 +0100

    remove now pointless separator handling from private::name();

diff --git a/config/kconfiggroup.cpp b/config/kconfiggroup.cpp
index 3c1cb79..873e285 100644
--- a/config/kconfiggroup.cpp
+++ b/config/kconfiggroup.cpp
@@ -92,7 +92,7 @@ class KConfigGroupPrivate : public QSharedData
     {
         if (mName.isEmpty())
             return "<default>";
-        return mName.mid(mName.lastIndexOf('\x1d') + 1);
+        return mName;
     }
 
     QByteArray fullName(const QByteArray& aGroup) const

commit 2ffec77e919712caceac2492d41f9151d87f3670
Author: Oswald Buddenhagen <ossi@ugly.local>
Date:   Thu Nov 22 20:18:34 2007 +0100

    escape group names so that they cannot be mistaken for immutability
    markers (or any other markers possibly added later).



[prev in list] [next in list] [prev in thread] [next in thread]