[RFC] KConfig API stuff

[Thomas Braxton]

On 10/21/07, Oswald Buddenhagen <ossi at kde.org> wrote:
>
> moin,
>
> as you probably know, puthuhn (bernhard loos) and me are fixing KConfig.
> while i'm pretty confident that nobody cares for the backend stuff we
> are doing, ;) there are also some API changes in the works.
> it's "a bit" late in the release process, so we made sure to get a
> preliminary OK from allen in advance.
> the additional effort for people not involved should be almost zero, as
> we do the complete porting - and this time you can expect quality work -
> after all, *i* am on it. ;)
> however, as this grows into unexpected dimensions, i'd like to discuss
> the changes in detail before we pursue this any further:


since you seem to think I'm a less than capable coder, I'll leave it to you.
After all you're the one with the kde.org address. But, I'll reply to this
email then I won't touch KConfig* again.

- kill kconfigbase, make kconfig inherit kconfiggroup.


It wasn't my idea, puthunh convinced me to readd it to the heirarchy.
kconfigbase was gone from my branch.

  - rationale: it is The Right Thing To Do (TM). the current asymmetry
>     of kconfig offering the group functions but no entry functions is
>     just awkward. the kconfig object itself is the <default> group (the
>     root group, or rather, the non-group) and should behave as such.


actually I would say that the kconfig object is not the default group, but
is rather the collection of *all* groups. groups also don't care about the
backend. this seems like making a collection of items a subclass of the
item. that's why kconfig offers the group functions, it is a collection of
groups. a group is a collection of entries, that's why kconfiggroup offers
the entry functions. the reason kconfiggroup also offers the group functions
is because a group can have subgroups.

  - problem: while at first it seemed like porting would be a trivial
>     search&replace in most cases, a "slight" problem turned up: kconfig
>     becomes implicitly shared, as kconfiggroup is. as a consequence, the
>     kconfiggroup constructors would take const KConfigGroup &master
>     instead of KConfigBase *master and a lot of explicit memory management
>     would need to be adapted.
>     so while as such this is definitely a feature, the porting effort is
>     just horrendous.
>   => without more helping hands, this is a no-go
>   - alternatives? maybe like this: kconfigbase dies and thus group
>     functions become unavailable via kconfig. no big loss: one can
>     already create a separate root group object - it would simply become
>     the only way. the porting effort should be *very* moderate.
>     - problems: it's uglier than the first variant
>     => no-go unless first option is unfeasible
> - renames in kconfiggroup: name to groupName, exists to groupExists,
>   rename in kconfig: name to configName
>   - rationale: clearly necessary with the changed hierarchy. does it
>     make sense without it?


if you say so. if they aren't going to have a common ancestor, the current
names are still valid, why would you ask a group what it's groupName is?
group.name() makes more sense than group.groupName(). config.name() makes
more sense than config.configName(). is there another name that they are
going to return in another function that requires the longer names?



>   => ?
> - rename kconfiggroup::clean back to to rollback.
>   - rationale: whoever renamed it had no idea of database terminology


you're right I don't know database terminology, but I know enough to know
that to rollback a db means to return to some previous state, which this
doesn't do. That's why I thought the name should be changed, to reflect what
it actually does and not pretend it does more.

  => go
> - remove kconfig::setGroup & kconfig::group
>   - rationale: the api sucks and is deprecated about forever.
>     additionally, in the current trunk it is outright defunct.
>   - problem: still some users left to port, but within reason
>   => still on go


agree

- remove kconfiggroup::changeGroup
>   - rationale: the api sucks - it is about the same as kconfig::setGroup
>   - problem: quite some users
>   => prolly no-go, will stay with deprecation


agree

- remove kconfiggroup::group
>   - rationale: it is just a weird way to call the constructor:


Actually I was thinking of going the other way, to make read-only groups
work correctly. make all the constructors protected except KConfigGroup() &
KConfigGroup(const KConfigGroup&).

    cg = KConfig(...).group("blah") => cg = KConfigGroup( KConfig(...),
> "blah" )
>     KConfigGroup cg( KConfig(...).group("blah") ) => KConfigGroup cg(
> KConfig(...), "blah" )
>   - problem: quite some users
>   => prolly no-go, will go with deprecation
> - remove kconfiggroup::sync

  - rationale: the api suggests a granularity that is simply not given.
>     cg.config().sync() is the proper call.


kinda agree

  - problems:
>     - quite some users. manageable, though.
>     - more verbose code. do we want it?
>   => ?
> - remove kconfiggroup::getConfigState


Again, another addition from puthuhn.

  - rationale: the api suggests a granularity that is simply not given.
>     cg.config().getConfigState() is the proper call.
>   => go
> - rename kconfig::getConfigState to accessMode
>   - rationale: states: noAccess, readOnly, readWrite ... no comment


agree

  => go
> - rename kconfiggroup::entryIsImmutable to isEntryImmutable
>   - rationale: better, right?


works either way.

  => go
> - move kconfiggroup::setReadDefaults to kconfig
>   - rationale: all users (mostly kconfigskeleton derivatives) deal with
> the
>     entire file at once anyway

  - problem: it's ugly and cries "i'm not reentrant"


another problem, how do you ensure that it is set on every group that comes
from a config object that is supposed to be reading defaults only?

  => no-go?
> - remove kconfig::setForceGlobal
>   - rationale: only user is kfile/kfilewidget.cpp. the alternative api
>     is passing the KConfigGroup::Global flag to writeEntry calls.


actually it's also set whenever "kdeglobals" is opened explicitely, so that
whoever is using it doesn't have to pass KConfigGroup::Global to every
writeEntry call.

  - alternatively, one might view it just like setReadDefaults and move
>     it to kconfiggroup instead.
>   => go?
> - remove kconfiggroup::NLS
>   - rationale: it is an alias for Localized. the internal KEntry uses
>     the Localized terminology, too, so for the sake of consistency ...
>   => go


either way.

- remove kconfig OnlyLocal and NoGlobals
>   - rationale: they are aliases for SimpleConfig resp. CascadeConfig.
>     it's more to learn and the negative meanings are potentially
>     confusing.


That's why I changed the names.

  => go
> - remove the separator argument from {read,write}Entry for lists
>   - rationale: the separator is a backend internal detail. with some
>     backends, it might be plain unchangeable.


agree

  - problems: none
>   - non-problems:
>     - there are some abuses of the separator argument and some plain
>       pointless uses. easily fixable.
>     - it was suggested that this would imply changing the kde-wide
>       separator back to the semicolon, as .desktop files use the
>       semicolon and kdesktopfile is in fact a kconfig, so we'd have no
>       way to get it right otherwise.
>       in fact, we might provide an api (possibly accessible by
>       kdesktopfile only) to put the entire file into .desktop mode -
>       that makes most sense anyway.
>       however, if somebody wants to go with semicolons kde-wide (hi
>       maelcum ;), the same as in my mail "KConfig list entries vs. kde3
>       vs. desktop entry spec" applies.
>   => clear go
> - remove kconfiggroup::entryMap
>   - rationale: the api sucks. it wants to return all values as strings,
>     which might be simply impossible without quite some additional magic
>     with some backends.


The only *hard* problem that I know of right now with returning strings is
when a stringlist has an entry that has backslash as it's last char confuses
KConfigGroup into thinking the separator is escaped when it shouldn't
be. I've tried to fix this multiple times, but you thought each was
sub-optimal.

I was thinking about changing this to return QMap<QString,QVariant>, when I
changed KEntry to use QVariant instead of QByteArray, but since I'm out of
this...

  - problems:
>     - the replacement api is keyList() plus readEntry(). however, when
>       one uses keyList() in the first place, it might mean that he has
>       no idea what type to expect from the entry. anyway,
>       returning/printing errors on individual readEntry() calls is
>       probably better than returning a partial map.
>     - the replacement is slower. hardly a problem, realistically
>       speaking.
>     - there are some users. probably sub-critical.
>   => go, maybe only deprecate instead.
>
> the code can be found under branches/work/kconfig_new_take2/ .
> note that not all changes are implemented yet and nothing beyond kdelibs
> is guaranteed to compile.
>
> --
> 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/20071021/6f9b4173/attachment.htm>