Extending KCModuleProxy

[Frans Englich]

On Friday 19 March 2004 21:15, Matthias Kretz wrote:
> Ok, I won't come around to finish going through that patch today so I
> rather send what I wrote so far...
>
> On Wednesday 17 March 2004 19:42, Frans Englich wrote:
> > True, the thing is, kcmoduleproxy.diff is 24kb big and the kcmoduleproxy.
> > {cpp,h} is 21.2kb in total - the class is basically rewritten.
> > realModule() is rewritten that's perhaps why it appears as moved(in total
> > 99 lines removed, 810 added). But I can run it through indent though,
> > should I?
>
> Almost. Still most of the old code that was in there didn't change. And
> it's nice if the diff would show what exactly was changed and what not.
> Having to manually look for those changes somehow defeats the point of a
> diff.

Yes, absolutely true. But it's irrelevant because there was basically no white 
space changes.

cat kcmoduleproxy.diff | grep "^+.*" | wc -l
    723
cat kcmoduleproxy.diff | grep "^-.*" | wc -l
     99

Judging from the patch realModule is heavily rewritten and accounts for most 
of the removed lines(28l), all that was retabbed was two 6-8 liner functions, 
showEvent and moduleChanged(16l in total) and the rest was header(30l) or 
misc. changes(~25). _AFAICT_
FYI, I did not change those two functions, if it's of any help.

>
> > > - - The changed signal is not identical to the one of the embedded KCM.
> > > The changed signal of the proxy is only emitted if the KCM really
> > > changed. If changed() == true and the KCM emits changed(true) the proxy
> > > won't emit the changed signal.
> >
> > Ok, that was my /intention/ too :)
> >
> > Sorry, you will have to be a little bit more specific. in realModule() I
> > do:
>
> I'm sorry this didn't come through as I meant: What I wanted to say was
> that the documentation for the changed signal was wrong where it says:
> "This signal is relayed from the encapsulated module, and is equivalent to
> the module's own changed(bool) signal."
> It was supposed to not be the same and has to, after your changes, still
> behave that way (which I think it does, from what I've seen).

Ups, could be a good idea to update the docs ;-)

>
> [snip the code I wrote myself ;-) ]
>
> > KControl's ConfigModule do lazy loading on ProxyWidget(which loads
> > ProxyView which loads a KCModule..). I would say the things KCModuleProxy
> > is supposed to do, is of interest for all cases where KCModules are
> > used(such as KControl).
>
> If I understand correctly KControl's code can be simplified using the new
> KCModuleProxy?

Yes, quite heavily. About 4-5 classes I think(which also would lead to feature 
gain). Dunno if it's worth doing before KDE 4 though, considering how much is 
going on in that area.

>
> > Yes, lazy loading was thrown out in my patch but it's back now(AFAICT).
> > It was easy to do - I merged realModule and init.
>
> Looks good.
>
> > The removal was intentional because I thought 95% of the cases one module
> > would be shown and the rest about 3-5 modules(configure dialogs). But
> > since it back, twisted games like in Kontact is possible. FYI, Kontact
> > without lazy loading is unacceptable slow.
>
> Even if it's only 3 modules. It still takes awefully long to load one of
> them - think of all the disk accesses. Lazy loading for KCMs is a _must_
> IMHO.

Yeah, and resources shouldn't be wasted just because it's "acceptable". 
Definitely a nice feature.

>
> > New versions attached(kstdgui committed). kcmultidialog.cpp#351 crashes
> > because KCModuleProxyPrivate* d; is not initialized. Which is really
> > weird to me since I do it in KCMultiDialog's constructors, AFAICT.
>
> I'll look into this if you give me more time. 

I found the problem. The pointer was zeroed in the init function...

> (I still know that code 
> pretty good. But I'm sick and can't concentrate...)

Hope you will recover! 


			Frans