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

Oswald Buddenhagen ossi at kde.org
Mon Nov 19 22:58:17 GMT 2007


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. :=)

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

> > - does kconfiggroup::keyList() behave as it should (i.e., not including
> >   keys from subgroups)?
> 
> Test added to kconfigtest, shows that keyList() behaves as it should.
> 
> > - and does kconfiggroup::entryMap()?
> 
> Same thing here.
> 
thanks

> --- 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 :=)

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