[PATCH] KCoreConfigSkeleton

[Oswald Buddenhagen]

On Sat, Nov 24, 2007 at 04:20:28PM -0700, Aaron J. Seigo wrote:
> On Saturday 24 November 2007, Oswald Buddenhagen wrote:
> > the nested groups are no problem as such - the code can be extended
> > to deal with it.
>
that's relatively simple. each item has an mGroup member. we can make
this a qstringlist instead of a qstring.

> > i don't see a fundamental issue with passing KConfigBase instead of
> > KConfig.
>
ok, and mGroup would become a relative location of the item. being a
stringlist, it could be empty as well.

> > however, this needs to be checked for side effects. i think at least
> > the kcfg file needs to mark those groups as dynamic, either entirely
> > dynamic or with some pattern the group name should comply with.
> > otherwise, for example, the legendary kcfgedit would try to put it
> > in the root of a file.
> 
> +1
> 
somebody wants to pick this up?

here are some observations from reading the code:
- each item having only a symbolic location encoded and constructing a
  configgroup on read/write is incredibly slow. ok, that's not going to
  be a problem in normal use, but maybe an alternative (storing a
  kconfiggroup) should be considered nonetheless.
- the typing system looks unnecessarily complex. i would think
  everything could be done via variants (after registering some custom
  types for passwords, paths, etc.). the type of the entry would be
  defined through the default, just like in kconfiggroup. all the
  conversion stuff is in kconfig already (or should be, fwiw).
- some minor things:
  - KConfigSkeletonItem::isEqual() looks highly suspicious. that's from
    the custom variant type camp, too, i think.
  - KConfigSkeletonItem::{min,max}Value are pointless for most types.
    how about KConfigSkeletonRangeItem?
  - KConfigSkeletonItem::readImmutability() looks completely out of
    place, particularly with the group argument. make isImmutable()
    virtual instead?
  - what is T & KConfigSkeletonGenericItem::value() good for? why isn't
    const T &value() + setValue() not good enough?
  - in KConfigSkeletonGenericItem::writeConfig is this line:
      if ( mReference != mLoadedValue ) // Is this needed?
    my reply is: no. it's even wrong. each item should have a dirty flag
    instead.
  - the readDefault implementation via kconfig::setReadDefaults is ugly
    by design, but i don't really have a much better idea that wouldn't
    add too much overhead to normal kconfig usage.

uha - i'm getting carried away again. ;)

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