Review Request: New Applet handle system

Giulio Camuffo giuliocamuffo at gmail.com
Wed Sep 1 17:15:31 CEST 2010



> On 2010-08-31 11:27:51, Marco Martin wrote:
> > i started with an api review of abstracthandle, will have to be done piece by piece because is really  monster patch.
> > (by the way, reviewboard still can't show the full diff, the patch should have been done with an old checkout probably)

the patch was against the last revision when i did it, but still i had problems trying to applying it to a checkout of that revision. don't know why.


> On 2010-08-31 11:27:51, Marco Martin wrote:
> > trunk/KDE/kdelibs/plasma/private/abstracthandle.h, line 67
> > <http://reviewboard.kde.org/r/5155/diff/1/?file=34663#file34663line67>
> >
> >     what is the exact use case for this function?

It does all the cleanings before destroying the handle. If you're asking me why i'm not doing them in the destructor then i'll say ask Kevin Ottens about it. I took that from the actual AppletHandle and i think there is a reason if there is it.


> On 2010-08-31 11:27:51, Marco Martin wrote:
> > trunk/KDE/kdelibs/plasma/private/abstracthandle.h, line 89
> > <http://reviewboard.kde.org/r/5155/diff/1/?file=34663#file34663line89>
> >
> >     isn't possible for the handle discover it by itself with hoverevents?

I'd say yes, but another time: why is it in AppletHandle? Maybe the Containment needs it.


> On 2010-08-31 11:27:51, Marco Martin wrote:
> > trunk/KDE/kdelibs/plasma/private/abstracthandle.h, line 123
> > <http://reviewboard.kde.org/r/5155/diff/1/?file=34663#file34663line123>
> >
> >     this is kinda necessary but i would love to move it out of the pubic api

Since it is called by the control elements i moved it in ControlElement, as a protected method.


- Giulio


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/5155/#review7311
-----------------------------------------------------------


On 2010-08-26 10:30:18, Giulio Camuffo wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/5155/
> -----------------------------------------------------------
> 
> (Updated 2010-08-26 10:30:18)
> 
> 
> Review request for Plasma and Aaron Seigo.
> 
> 
> Summary
> -------
> 
> This is a rewamp of the Applet handle system. Through its modular architecture it easily allows modifications and reuse of code.
> 
> It features a base Handle class, AbstractHandle, and a base class for the control elements, ControlElement. I developed an handle based on the actual AppletHandle, DesktopHandle, and the control elements for the usual operations.
> 
> 
> Diffs
> -----
> 
>   trunk/KDE/kdelibs/plasma/CMakeLists.txt 1168271 
>   trunk/KDE/kdelibs/plasma/applet.h 1168271 
>   trunk/KDE/kdelibs/plasma/applet.cpp 1168271 
>   trunk/KDE/kdelibs/plasma/containment.h 1168271 
>   trunk/KDE/kdelibs/plasma/containment.cpp 1168271 
>   trunk/KDE/kdelibs/plasma/extenders/extender.cpp 1168271 
>   trunk/KDE/kdelibs/plasma/extenders/extenderitem.cpp 1168271 
>   trunk/KDE/kdelibs/plasma/private/abstracthandle.h PRE-CREATION 
>   trunk/KDE/kdelibs/plasma/private/abstracthandle.cpp PRE-CREATION 
>   trunk/KDE/kdelibs/plasma/private/applet_p.h 1168271 
>   trunk/KDE/kdelibs/plasma/private/applethandle.cpp 1168271 
>   trunk/KDE/kdelibs/plasma/private/applethandle_p.h 1168271 
>   trunk/KDE/kdelibs/plasma/private/configurecontrol.h PRE-CREATION 
>   trunk/KDE/kdelibs/plasma/private/configurecontrol.cpp PRE-CREATION 
>   trunk/KDE/kdelibs/plasma/private/containment_p.h 1168271 
>   trunk/KDE/kdelibs/plasma/private/controlelement.h PRE-CREATION 
>   trunk/KDE/kdelibs/plasma/private/controlelement.cpp PRE-CREATION 
>   trunk/KDE/kdelibs/plasma/private/controlelement_p.h PRE-CREATION 
>   trunk/KDE/kdelibs/plasma/private/desktophandle.h PRE-CREATION 
>   trunk/KDE/kdelibs/plasma/private/desktophandle.cpp PRE-CREATION 
>   trunk/KDE/kdelibs/plasma/private/maximizecontrol.h PRE-CREATION 
>   trunk/KDE/kdelibs/plasma/private/maximizecontrol.cpp PRE-CREATION 
>   trunk/KDE/kdelibs/plasma/private/movecontrol.h PRE-CREATION 
>   trunk/KDE/kdelibs/plasma/private/movecontrol.cpp PRE-CREATION 
>   trunk/KDE/kdelibs/plasma/private/removecontrol.h PRE-CREATION 
>   trunk/KDE/kdelibs/plasma/private/removecontrol.cpp PRE-CREATION 
>   trunk/KDE/kdelibs/plasma/private/resizecontrol.h PRE-CREATION 
>   trunk/KDE/kdelibs/plasma/private/resizecontrol.cpp PRE-CREATION 
>   trunk/KDE/kdelibs/plasma/private/rotatecontrol.h PRE-CREATION 
>   trunk/KDE/kdelibs/plasma/private/rotatecontrol.cpp PRE-CREATION 
> 
> Diff: http://reviewboard.kde.org/r/5155/diff
> 
> 
> Testing
> -------
> 
> It isn't finished. It's missing the touch events management (which, however, it's hard to me to do, 'cause i don't have any touch screen device) and a better drag and drop system between containments. I'd like, however, to know what you think about what i've done, especially about the architecture.
> 
> What's here works, though.
> 
> 
> Thanks,
> 
> Giulio
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/plasma-devel/attachments/20100901/757741fe/attachment.htm 


More information about the Plasma-devel mailing list