[Panel-devel] Combining startup notifications and tasks

[Aaron J. Seigo]

On Wednesday 19 December 2007, Jason Stubbs wrote:
> On Wednesday 19 December 2007 21:24:35 Jason Stubbs wrote:
> > The other patch ... uses the method added by the above patch
> > to check whether a taskAdded() notification matches any of the existing
> > startup items.
> >
> > By the way, the patches make http://bugs.kde.org/show_bug.cgi?id=154147
> > more pronounced but I'll be looking at that next. :)
>
> It was making it more pronounced because there was a bug where the task was
> being readded to the root group. This fixes that and now I can't reproduce
> the above bug. But at minimum, *something* should be ensuring that adding
> twice doesn't cause an issue although I'm still to find it..

the annoyance with this approach is that now the WindowTaskItem class will get 
filled with "if (startup)" type code. we had this situation in kicker's 
taskbar as well and it doesn't make for pretty code.

i see why you're doing what you've done here (re-use the existing startup task 
graphical item) ... i wonder if it couldn't simply replace the startup item 
with the new task item rather than make them the same literal object.

however, without creating another class that is the actual QGraphicsItem and 
turning AbstractTaskItem into a delegate-like class that does the painting, i 
can't think of a better way of doing this. with that approach, one would have 
task bar entries that would just sit there taking up space with the delegate 
doing all the management; this would allow re-use of the graphical element 
(avoiding the visual add/remove problems your patch addresses) while keeping 
the actual visualization code separated.

going the delegate-does-the-painting route is probably the best solution, but 
is a fair amount more coding and changing things around. your patch is ok for 
now, but perhaps you could put a comment in the code at an appropriate point 
about this? =)

> The taskmanager patch is fine as is.

i don't like that the constructor has yet another parameter, however. would be 
nicer to see a setStartupId(const KStartupInfoId &) instead.

keeping the startupid around forever is also a bit unfortunate.

hm... looking at the patch i think i'd actually do this a bit differently in 
libtaskmanager. my assumption is that startup tasks are shortlived and few so 
can be more wasteful; tasks should be conservative as they are long lived and 
potentially quite plentiful.

so i'd instead add a list to Startup of related windowIDs (or even TaskPtrs?). 
then one simply calls startup->matches(task); and it looks in its internal 
list.

when creating a new task, it would check for a matching startup (your patch 
already does that), find the Startup::Ptr (if any) related to that and 
associate the task with it.

this way the bookkeeping information is only kept around as long as the 
startup item is in libtaskmanager. which makes more sense to me at least.

thoughts?

(oh, btw, instead of moving the whole Startup class in the header, you could 
have simply forward declared it with "class Startup;" =)

-- 
Aaron J. Seigo
humru othro a kohnu se
GPG Fingerprint: 8B8B 2209 0C6F 7C47 B1EA  EE75 D6B7 2EB1 A7F1 DB43

KDE core developer sponsored by Trolltech
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
Url : http://mail.kde.org/pipermail/panel-devel/attachments/20071219/f719e772/attachment.pgp