even more on kconfig escapes (Re: KDE/kdelibs/kdeui/icons)

Andreas Pakulat apaku at gmx.de
Thu Nov 22 23:54:21 GMT 2007


On 22.11.07 17:05:06, Thomas Braxton wrote:
> On 11/22/07, Andreas Pakulat <apaku at gmx.de> wrote:
> > On 22.11.07 16:36:06, Thomas Braxton wrote:
> > > 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.
> >
> > No, name() should return only the name of this group, not any parent
> > group. But anyway Osswald already has a patch posted that fixes nested
> > groups the right way.
> mName doesn't contain the parent name, it never has, that's why it broke.

It does, this:

KConfigGroup foobar(grp, "foo/bar");

doesn't create a config group name "foo/bar", which can easily be
observed by doing 

KConfigGroup foo(grp,"foo");
KConfigGroup bar(&foo, "bar");

Both foobar and bar will read and write to the same "place". Thats the
real problem that needs fixing, my change just let this surface.

> > > 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.
> and if you have a group that *should* have slashes this will break.

You can't unless you encode the slashes, which didn't happen until now.

> Changing the separator is the only way to fix this.

No, proper encoding is another way to fix this. But of course thats a
problem if you want to stay backwards-compatible, at least for reading.

> > 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.

Well, guess what I tested that and it doesn't work. Specifically it
broke at least one of the tests. The problem is what I said above
"foo/bar" wasn't considered a group of its own, or rather is considered
the same as the subgroup "bar" in the group "foo".

Andreas

-- 
A day for firm decisions!!!!!  Or is it?




More information about the kde-core-devel mailing list