-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/1526/
-----------------------------------------------------------
(Updated 2009-11-24 22:21:35.429523)
Review request for Plasma, Aaron Seigo and Marco Martin.
Changes
-------
Addresses all the points from the review of aaron (thanks for the review=).
I cannot check it thoroughly since my tasks applet crashes as soon as i have to
reload a group item (crash somwhere in qt code).
This happens also with the applet from trunk, so i guess i have to investigate
into qt once....
To avoid a memory leak i stopped clearing the groups (since anyway we reload
the tasks, right?)
Normally the empty groups are closed by the grouping strategy, the rootgroups
will be destroyed as the groupmanager is destroyed since they are QObject
children of the groupmanager.
I will do some further testing as soon as i manage to fix my setup.
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 (updated)
-----
/trunk/KDE/kdebase/workspace/libs/taskmanager/abstractgroupableitem.h 1053463
/trunk/KDE/kdebase/workspace/libs/taskmanager/abstractgroupableitem.cpp
1053463
/trunk/KDE/kdebase/workspace/libs/taskmanager/abstractgroupingstrategy.h
1053463
/trunk/KDE/kdebase/workspace/libs/taskmanager/abstractgroupingstrategy.cpp
1053463
/trunk/KDE/kdebase/workspace/libs/taskmanager/abstractsortingstrategy.h
1053463
/trunk/KDE/kdebase/workspace/libs/taskmanager/abstractsortingstrategy.cpp
1053463
/trunk/KDE/kdebase/workspace/libs/taskmanager/groupmanager.h 1053463
/trunk/KDE/kdebase/workspace/libs/taskmanager/groupmanager.cpp 1053463
/trunk/KDE/kdebase/workspace/libs/taskmanager/strategies/manualgroupingstrategy.h
1053463
/trunk/KDE/kdebase/workspace/libs/taskmanager/strategies/manualgroupingstrategy.cpp
1053463
/trunk/KDE/kdebase/workspace/libs/taskmanager/strategies/manualsortingstrategy.h
1053463
/trunk/KDE/kdebase/workspace/libs/taskmanager/strategies/manualsortingstrategy.cpp
1053463
/trunk/KDE/kdebase/workspace/libs/taskmanager/taskgroup.h 1053463
/trunk/KDE/kdebase/workspace/libs/taskmanager/taskgroup.cpp 1053463
/trunk/KDE/kdebase/workspace/libs/taskmanager/taskitem.h 1053463
/trunk/KDE/kdebase/workspace/libs/taskmanager/taskitem.cpp 1053463
/trunk/KDE/kdebase/workspace/plasma/desktop/applets/tasks/tasks.h 1053463
/trunk/KDE/kdebase/workspace/plasma/desktop/applets/tasks/tasks.cpp 1053463
Diff: http://reviewboard.kde.org/r/1526/diff
Testing
-------
Tried it, works.
Thanks,
Christian
_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel
