KConfigSkeleton::usrWriteConfig()

[Andreas Pakulat]

On 22.04.07 13:35:27, Cornelius Schumacher wrote:
> On Sunday 22 April 2007 13:00, Andreas Pakulat wrote:
> > On 22.04.07 12:21:12, Cornelius Schumacher wrote:
> > > This commit
> > >
> > > http://websvn.kde.org/trunk/KDE/kdelibs/kdeui/config/kconfigskeleton.cpp?
> > >r1=559982&r2=560326
> > >
> > > moved the code to write KConfigSkeleton's configuration data from the
> > > writeConfig() function to the virtual usrWriteConfig() function. This
> > > broke all applications which overwrote usrWriteConfig(), e.g. KOrganizer.
> > >
> > > Is there any reason why this was done?
> >
> > Thats in the commit message, the reason is to be able to completely
> > replace the logic that was formerly in writeConfig. And this is needed
> > at least in KDevelop4.
> 
> Why? What is KDevelop doing there?

For usrWriteConfig it only does something after the config has been
written (namely upload it to the remote project location). But for the
default and read methods it works on 2 config files and depending on
their content uses the values from one or the other (one contains the
"defaults", while the other has developer specific items).

> In my opinion replacing the logic and extending it are two different things. 
> If both is required we should have different hooks.

Why, if you can do the same in just one function. Also in the write-case
in kdevelop we need to be sure that our extended code is run _after_ the
default code, in the case of readConfig we need to make sure our special
code is run _before_ the default code. The only way to provide this is
by having the customized usrWriteConfig function dictate when the
KConfigSkeleton implementation is called. Others may have other needs
for the order.

> > > The usrWriteConfig() function was
> > > supposed to be a hook for customizations, not a function doing vital
> > > stuff. Any objections to reverting that change?
> >
> > Yes, as Thomas already said you can just call the base class
> > implementation and be done with it.
> 
> Sure, but there are several problems with that. First is that an unknown 
> number of applications is now broken which relied on the previous behavior. 

No offense meant, but those applications have been broken since quite
some time (the change was done 10 months ago). If nobody noticed thats
really bad.

> Second this is a cause for subtle errors as it's easy to forget or not know 
> in the first place, that a reimplementation has to call the base class 
> implementation there, especially as it's not documented.

Well, writeConfig()'s documentation says clearly that its implemented by
usrWriteConfig(). I guess this could use another note as you said and it
should probably also be mentioned in KDE4Porting.html.

> Third, as I said above, it's mixing different needs and concepts, so
> the API is less straightforward now.

IMHO its not. I have a special need for writing configs, so I implement
usrWriteConfig. I realize I also need the default implementation, so I
call that at a given time. To me this is quite natural.

Andreas

-- 
Are you making all this up as you go along?