[PATCH]KConfigGroup::groupList() kind of broken

Andreas Pakulat apaku at gmx.de
Mon Nov 19 23:13:01 GMT 2007


On 19.11.07 23:58:17, Oswald Buddenhagen wrote:
> On Mon, Nov 19, 2007 at 11:36:48PM +0100, Andreas Pakulat wrote:
> > On 19.11.07 23:12:19, Oswald Buddenhagen wrote:
> > > On Mon, Nov 19, 2007 at 10:12:25PM +0100, Andreas Pakulat wrote:
> > > > it seems that KConfigGroup::groupList() doesn't work really well
> > > > with subgroups.
> > > >
> > > yes. i noticed that before. i'm somewhat reluctant to fix it now,
> > > though - it could break code silently. otoh, it's just wrong and
> > > probably not relied much upon.
> > 
> > I wonder wether nested groups are used that much at all, I mean its a
> > pretty "new" feature
> 
> > and its not advertised in the api dox.
> > 
> ah, that's what i needed to hear. :=)

I'm probably going to change that after comitting this :)

> > > more on that matter:
> > > - kconfiggroup::name() returns the full path, too. while that can be
> > >   useful, it should be a separate function.
> > 
> > Indeed, and in fact one of the Private classes already has "fullName()".
> 
> > I'm not sure wether name() should be changed now and then a new function
> > added to KConfigGroup in 4.1... Opinions?
> > 
> good idea, also precludes silent failures.

So are you saying we should change name() now to make sure we don't get
silent failures later on and introduce a new function for KDE 4.1?

> > --- config/kconfig.cpp	(Revision 738799)
> > +++ config/kconfig.cpp	(Arbeitskopie)
> > @@ -167,13 +167,16 @@
> >  
> >  QStringList KConfigPrivate::groupList(const QByteArray& group) const
> >  {
> > -    QStringList groups;
> > +    QSet<QString> groups;
> >  
> >      foreach (const KEntryKey& key, entryMap.keys())
> >          if (key.mKey.isNull() && key.mGroup.startsWith(group) && key.mGroup != group)
> > -            groups << QString::fromUtf8(key.mGroup);
> > +	{
> > +            QString groupname = QString::fromUtf8(key.mGroup.mid(group.length()+1));
> > +            groups << groupname.left(groupname.indexOf("/"));
> > +	}
> >  
> > -    return groups;
> > +    return groups.toList();
> >  }
> >  
> +1
> (well, you have some tabs in here, for which you will be shot if you
> commit it as is, but that's another story :=)

Damn, I hate vim for doing this to me :) If just kdevelop3 would be a
bit better at working with kdelibs...

Of course will fix the indentations before comitting.

Andreas

-- 
You will be the victim of a bizarre joke.




More information about the kde-core-devel mailing list