[PATCH] KConfig code cleanups

[Thomas Braxton]

On 10/28/07, Oswald Buddenhagen <ossi at kde.org> wrote:
>
> please teach your mailer to attach patches as text not binary.


sorry, gmail does what it wants

On Sat, Oct 27, 2007 at 05:07:10PM -0500, Thomas Braxton wrote:
> > 1) do we want to support an empty group name? We've never supported it
> > before, and I really don't see a use for it, but it shouldn't be too
> hard to
> > add support for.
> > 1a) If we do, do we want to support empty sub-group names?
> > 2) this one depends on #1, should the name of the default group be
> exposed?
> > It's really just an implementation detail that I think should never be
> > returned to the user, unless we decide to support empty group names.
> >
> the concept of giving the root group a virtual name is bogus. it should
> be a null string. we planned to eliminate this <default> madness - a
> list of all(?) occurrences can be found here:
> http://web.davidfaure.fr/kde/default.list - wanna go ahead with it?
> this, btw, does not mean that the name of the root group is "" - the
> root group simply has no name, but we need to address it somehow through
> our tree-wise inferior api. consequently, subgroups cannot have empty
> names, either.


A couple of problems with what you wrote:
1) A group with an empty name does not have to be the <default> group, it
would be written to an ini file as [].
2) The unnamed <default> group is written to the ini file without a group
marker at all. The other toplevel groups are siblings not children of the
unnamed <default> group, so there is no *root* group.


I was referring to this thread
http://lists.kde.org/?l=kde-core-devel&m=119151900709470&w=2
So do we want to support "[]"? If yes, then what should be returned as the
name of the <default> group? QByteArray()? If not, then why not use the name
""?



> > 3) should the translated entries from other locales be removed from the
> > entryMap?
> >
> yes
>
> > Index: kconfig.cpp
> > ===================================================================
> > --- kconfig.cpp       (revision 730036)
> > +++ kconfig.cpp       (working copy)
> > @@ -520,7 +520,9 @@
> >  bool KConfig::isGroupImmutableImpl(const QByteArray& aGroup) const
> >  {
> >      Q_D(const KConfig);
> > -    return isImmutable()|d->entryMap.getEntryOption(aGroup, 0, 0,
> KEntryMap::EntryImmutable);
> > +    if (aGroup.isEmpty())
> > +        return d->bFileImmutable;
> > +    return d->bFileImmutable|d->entryMap.getEntryOption(aGroup, 0, 0,
> KEntryMap::EntryImmutable);
> >  }
> >
> this looks bogus. it should be possible to have an empty group with
> an immutability marker. fix the entry map if it doesn't work.



the <default> group is immutable if the file is immutable, since it doesn't
have it's own group marker in the file, but then if you want to support [],
then you're right the "" group will have it's own immutability marker.

> Index: kconfiggroup.cpp
> > ===================================================================
> > --- kconfiggroup.cpp  (revision 730036)
> > +++ kconfiggroup.cpp  (working copy)
> > @@ -1096,7 +1129,6 @@
> >  {
> >      writePathEntry(pKey.toUtf8(), path, pFlags);
> >  }
> > -
> >  void KConfigGroup::writePathEntry(const char *pKey, const QString &
> path, WriteConfigFlags pFlags)
> >  {
> >      writePathEntry(QByteArray(pKey), path, pFlags);
> >
> not good. ;)


oops

the rest seems ok to me.
>
> --
> Hi! I'm a .signature virus! Copy me into your ~/.signature, please!
>
> --
> Chaos, panic, and disorder - my work here is done.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20071028/46c6dc0c/attachment.htm>