Extending KCModuleProxy

[Frans Englich]

On Tuesday 16 March 2004 14:27, Matthias Kretz wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Hi Frans,
>
> first of all thank you for documenting the KCModuleProxy class.
>
> A few remarks on what I've seen (I've only looked through the diff - no
> testing done):
> - - Reading the diff was really hard because you reordered the code and
> changed the indenting. So I had to manually diff the old and new code :-(
> - - Please don't remove the vim modeline. People using vim automatically
> will use the right indentation style if it's present.

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?

> - - 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:

connect( d->kcm, SIGNAL( changed( bool ) ),
    SLOT(moduleChanged(bool)) );
 
And moduleChanged looks like this:

void KCModuleProxy::moduleChanged( bool c )
{
        if( d && d->changed != c )
        {
                d->changed = c;
                emit changed( c );
                emit changed( this );
        }
}


> - - there's an i18n call missing when creating the KCMError object

Fixed.

> - - The proxy was supposed to be really light weight. Just think of the
> Kontact config dlg which has sth. like 30 KCMs or so.
> The proxy makes lazy 
> loading easy and without a lot of overhead. The new implementation doesn't
> look very optimised on that regard (If I read correctly it even doesn't do
> lazy loading anymore). I'd like to see some numbers like how long it takes
> to create the Kontact config dlg.
>
> before I can continue to make usefull comments I'd need to know if you
> really did throw out lazy loading...
>
> JFYI: KCModuleProxy was supposed to do two things:
> 1. sane changed handling
> 2. easy lazy loading
> This is very different from what the ProxyWidget in KControl does so the
> idea never occured to me to merge the two.
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).

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.

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.

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.



Thanks,

		Frans



-------------- next part --------------
A non-text attachment was scrubbed...
Name: kcmoduleproxy.diff
Type: text/x-diff
Size: 24086 bytes
Desc: not available
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20040317/bd0c1a40/attachment.diff>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: kcmultidialog.diff
Type: text/x-diff
Size: 6348 bytes
Desc: not available
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20040317/bd0c1a40/attachment-0001.diff>