Review Request: Fix taskmanager's "by desktop" sorting mode

[Aaron Seigo]

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

Ship it!


some small comments, but it looks pretty good. i'd say address the two issues below in a satisfactory manner and then commit.


trunk/KDE/kdebase/workspace/libs/taskmanager/abstractgroupableitem.h
<http://reviewboard.kde.org/r/3375/#comment4264>

    agreed; it probably shouldn't be virtual. if needed, a setId(int) could always be added later on, but for now the guarantee of uniqueness is probably the most important thing.



trunk/KDE/kdebase/workspace/libs/taskmanager/strategies/desktopsortingstrategy.cpp
<http://reviewboard.kde.org/r/3375/#comment4265>

    all of the winIds() stuff can be gotten rid of now that there is the id() method. if winIds().isEmpty(), then desktop() is going to be 0. and since the winIds() list isn't beig used, then it's probably enough to just do this:
    
    const int leftDesktop = left->desktop();
    const int rightDesktop = right->desktop();
    if (leftDesktop() == rightDesktop) {
        return left->id() < right->id();
    }
    
    return leftDesktop < rightDesktop;


- Aaron


On 2010-03-29 17:08:22, Dmitry Suzdalev wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/3375/
> -----------------------------------------------------------
> 
> (Updated 2010-03-29 17:08:22)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> -------
> 
> This patch fixes a sorting order for "Sort by Desktop" mode of taskmanager lib.
> 
> Summary:
> When in "Sort by Desktop" mode, sort by_desktop+by_winId, instead of by_desktop+by_winTitle as it is now.
> 
> More details:
> Currently in "by desktop" sorting mode tasks are sorted by desktop and then by name.
> This leads to inconvenient things, here are some examples:
> 
> - I have a browser with several tabs open. Whenever I change a tab, browser changes window title.
> This makes task jump in a task bar from one position to another while I'm simply changing tabs.
> - I have a 'konsole' window and as I do 'cd onedir', 'cd zletter_dir', etc, title is changed, task jumps
> - Some other situations caused this too, don't recall, but you got the idea :)
> 
> What I've done:
> Instead of sorting by name, i made it to sort by winId. Tasks without winId are sorted out to the end of the list
> and sorted by name there. Typically these are the startup-in-progress items.
> 
> As a side effect this fixes bug https://bugs.kde.org/show_bug.cgi?id=219528 which has the same roots:
> invalid sorting order due to wrong comparison of regular items with "starting up" items.
> 
> Long description, short patch ;)
> 
> 
> This addresses bug 219528.
>     https://bugs.kde.org/show_bug.cgi?id=219528
> 
> 
> Diffs
> -----
> 
>   trunk/KDE/kdebase/workspace/libs/taskmanager/abstractgroupableitem.h 1105271 
>   trunk/KDE/kdebase/workspace/libs/taskmanager/abstractgroupableitem.cpp 1105271 
>   trunk/KDE/kdebase/workspace/libs/taskmanager/strategies/desktopsortingstrategy.cpp 1105271 
> 
> Diff: http://reviewboard.kde.org/r/3375/diff
> 
> 
> Testing
> -------
> 
> Tested on trunk. Task items retain their sort order, not reacting to title changes, charming :)
> 
> It affects only task applets with sort mode set to "by desktop"
> 
> 
> Thanks,
> 
> Dmitry
> 
>