fix my KCModule issues once and for all

[Roger Larsson]

On Thursday 25 September 2003 19.32, Matthias Kretz wrote:
> On Thursday September 25 2003 18:09, Roger Larsson wrote:
> > On Wednesday 24 September 2003 19.06, Matthias Kretz wrote:
> > > I think the second solution is nicer and should work out better in the
> > > long run. But if you're interested in the first patch - or even think
> > > that it's better let me know.
> >
> > I like it :-)
>
> I guess you mean the second patch?
>
> > If it can be used as a Widget, why not call it KCMWidget?
>
> Because Proxy or Decorator are known patterns and people might understand
> easier what it does. Widget on the other hand is not very descriptive since
> a KCM is a widget, too.
>
> > BTW this code - is this OK?
>
> Well, after I commit my patch the "emit changed" is done automatically -
> KControl does it itself and KCMultiDialog uses KCModuleDecorator to do it.
> So after a load() or save() "emit changed( false )" is called. I don't call
> "emit changed( true )" after defaults(), because it could very well be that
> the current settings are the defaults - so pressing the button didn't
> change any settings. I'm not sure if it's a good thing to do usability wise
> since the user might expect the apply button to be enabled after clicking
> "Defaults" (so this is up for discussion).

Not good.

>
> In the rare case that saving failed and the KCM would like to keep a
> changed state you would call:
> QTimer::singleShot( 0, this, slotEmitChanged() );

Even worse! :-(

>
> with that slot calling "emit changed( true );".
>

Rule: The Decorator should - heal, support but never hurt

> So for the KDevelop template I think you can just leave out the "emit
> changed" from the save(), load() and defaults() methods (because that's
> what I am trying to fix with my patches - that those calls are not
> necessary anymore).

It is not mine - I do not even have cvs write access...

But I think that whatever the final solution is - kdevelop template SHOULD
be correct. What is worse is that kdevelop templates might need to work for
older KDE versions.

So what was really wrong with adding "setChanged( ... );" where needed?

Could the Decoration use a second object and compare? Something like this?
	
load()
{
 	KCModule *temp = d->kcm;
	d->kcm = new KCModuleDecoratorPrivate( info );
	d->kcm->load();
	emit changed(*d->kcm != *temp);
	delete temp;
}

save()
{
	KCModule *temp =  new KCModuleDecoratorPrivate( info );
	d->kcm->save();
	temp->load(); // loads whatever is saved
	emit changed(*d->kcm != *temp);
	delete temp;
}
	
default()
{
	KCModule *temp =  d->kcm;
	d->kcm = new KCModuleDecoratorPrivate( info ); // == Default?
	d->kcm->default(); // needed?
	emit changed(*d->kcm != *temp);
	delete temp;
}

-- 
Roger Larsson
Skellefteå
Sweden