Review Request: taskmanager library: Manual Sorting Fix

[Beat Wolf]

> On 2009-09-04 20:16:52, Aaron Seigo wrote:
> > this results in a "leak" in that every window ever created will have an item that stays forever, no? shouldn't it only keep track of winIds that still exist, and do so in the manual grouping strategy?
> 
> Christian Mollekopf wrote:
>     Yes this would result in a "leak" (as long the groupmanager instance exists).
>     I just noticed that also manual grouping is broken since it also relies on the pointers to remain.
>     
>     I will work on a new patch where manual grouping and sorting keep track of the windows/groups by winIds.
> 
> Aaron Seigo wrote:
>     any progress on this?
> 
> Christian Mollekopf wrote:
>     yes, but since i'm rather busy atm. it takes some more time.
>     I'll try to finish it until next week.
>     
>     Cheers
> 
> jedd wrote:
>     Out of curiosity, will this change affect how new applications are displayed/handled in the taskbar?  Ideally they'd just appear (and stay unless moved) to the right of any extant tasks.  If not, can I ask a big favour ... ? ;)

any work on this patch? would be nice to have one of the more reported taskmanager bugs fixed for 4.4


- Beat


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


On 2009-10-19 21:10:07, Christian Mollekopf wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/1526/
> -----------------------------------------------------------
> 
> (Updated 2009-10-19 21:10:07)
> 
> 
> Review request for Plasma, Aaron Seigo and Marco Martin.
> 
> 
> Summary
> -------
> 
> this fixes the manual sorting strategy, which is broken atm if the desktop is changed.
> 
> Since the manual sorting strategy relies on AbstractGroupableItem pointer not to change, we cannot remove it from the bookkeeping in case it returns (after a desktop change for instance).
> I don't know if this is acceptable because this results in the item never being removed from the itemList until the groupmanager instance is deleted (lifetime of the applet which created the instance).
> 
> Another option would be to identify tasks and groups by WId, which is a bit more complicated, but if you think it would be better/cleaner, i could supply a patch.
> 
> 
> This addresses bug 200255.
>     https://bugs.kde.org/show_bug.cgi?id=200255
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdebase/workspace/plasma/desktop/applets/tasks/tasks.h 1034424 
>   /trunk/KDE/kdebase/workspace/plasma/desktop/applets/tasks/tasks.cpp 1034424 
>   /trunk/KDE/kdebase/workspace/libs/taskmanager/taskitem.h 1034424 
>   /trunk/KDE/kdebase/workspace/libs/taskmanager/taskitem.cpp 1034424 
>   /trunk/KDE/kdebase/workspace/libs/taskmanager/strategies/manualsortingstrategy.h 1034424 
>   /trunk/KDE/kdebase/workspace/libs/taskmanager/strategies/manualsortingstrategy.cpp 1034424 
>   /trunk/KDE/kdebase/workspace/libs/taskmanager/taskgroup.h 1034424 
>   /trunk/KDE/kdebase/workspace/libs/taskmanager/taskgroup.cpp 1034424 
>   /trunk/KDE/kdebase/workspace/libs/taskmanager/strategies/manualgroupingstrategy.h 1034424 
>   /trunk/KDE/kdebase/workspace/libs/taskmanager/strategies/manualgroupingstrategy.cpp 1034424 
>   /trunk/KDE/kdebase/workspace/libs/taskmanager/abstractsortingstrategy.cpp 1034424 
>   /trunk/KDE/kdebase/workspace/libs/taskmanager/groupmanager.h 1034424 
>   /trunk/KDE/kdebase/workspace/libs/taskmanager/groupmanager.cpp 1034424 
>   /trunk/KDE/kdebase/workspace/libs/taskmanager/abstractsortingstrategy.h 1034424 
>   /trunk/KDE/kdebase/workspace/libs/taskmanager/abstractgroupingstrategy.cpp 1034424 
>   /trunk/KDE/kdebase/workspace/libs/taskmanager/abstractgroupableitem.h 1034424 
>   /trunk/KDE/kdebase/workspace/libs/taskmanager/abstractgroupingstrategy.h 1034424 
> 
> Diff: http://reviewboard.kde.org/r/1526/diff
> 
> 
> Testing
> -------
> 
> Tried it, works.
> 
> 
> Thanks,
> 
> Christian
> 
>