Review Request: ScriptEngine's as Containments

[Sebastian Sauer]

On Wednesday 05 March 2008, Aaron J. Seigo wrote:
> isn't actually needed since that property is defined in another .desktop
> file already. if it's already been registered in one .dekstop file, all of
> them can use it. i wasn't aware of that interesting detail either until
> shortly before 4.0...

ah, that's new to me too :)

> also, this:
>
> +        offers = KServiceTypeTrader::self()->query("Plasma/Containment",
> constraint);
> +        isContainment = true;
> +        if(!offers.isEmpty()) {
> +            kDebug() << "offers is empty for " << appletName;
> +            return 0;
> +        }
>
> in applet.cpp shouldn't be necessary; since all containments also have
> Plasma/Applet in their service types list, it will never get to this
> branch.

It does in the case of SK which does not implement an Applet or Containment 
(or libs for any of them) but only a AppletScript that operates on the 
created "null" Applet/Containment depending on what the X-KDE-ServiceTypes 
defines.

> so with the second check for Plasma/Containment, we could change
> their .desktop files to not have
> X-KDE-ServiceType=Plasma/Applet,Plasma/Containment as they do now, but
> rather just X-KDE-ServiceType=Plasma/Containment. this would make it easy
> to sort out the "widgets-that-are-containments-too" from the "just
> containments" entries.
>
> neat; i like that =)

heh. I was beliving that this is/was the intention since the design for it was 
already in place what also explains the;

> it's impressive how little the code needed to be changed ..... coo'

y, most of the time was needed to figure out, that the containment desktopfile 
wasn't installed (as usual the most easy things eat most of the time ;)

> so other than the .desktop file change which isn't necessary, it looks good
> to go.

ok, thanks :)