fix my KCModule issues once and for all
Roger Larsson
roger.larsson at norran.net
Thu Sep 25 19:43:44 BST 2003
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
More information about the kde-core-devel
mailing list