Review Request: windows port of taskmanager library

[Aaron Seigo]

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


with a bit more work, the TaskManager class could also be made ifdef free. there are two blocks of code that need to be ifdef'd as far as i can see, and in them they check for just a few things: is the window set to skip the pager? is the window mapped? should the window be included in the task manager? a similar approach taken with Task could be taken with TaskManager. personally, i far prefer separating out the platform specific code like that; ifdef's tend to lead to untested branches and compile errors due to bit rot.

anyways, with a few more changes (noted below) this can go in. thanks for the patch :)


trunk/KDE/kdebase/workspace/libs/taskmanager/task.h
<http://reviewboard.kde.org/r/1101/#comment1236>

    this could easily be changed to:
    
    void addTransient(WId w, bool demandingAttention)
    
    the only thing that the NETWinInfo object is used for in addTransient is to check for attention demanding.
    
    that would allow addTransient to be in both unix and win builds and one less ifdef.



trunk/KDE/kdebase/workspace/libs/taskmanager/task.cpp
<http://reviewboard.kde.org/r/1101/#comment1233>

    agreed



trunk/KDE/kdebase/workspace/libs/taskmanager/task.cpp
<http://reviewboard.kde.org/r/1101/#comment1232>

    instead of #include'ing the files here, this should be done in the CMakeLists.txt file



trunk/KDE/kdebase/workspace/libs/taskmanager/taskmanager.cpp
<http://reviewboard.kde.org/r/1101/#comment1235>

    is this needed?



trunk/KDE/kdebase/workspace/libs/taskmanager/taskmanager.cpp
<http://reviewboard.kde.org/r/1101/#comment1234>

    why is this #ifndef'd?


- Aaron


On 2009-07-22 23:57:29, Patrick Spendrin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/1101/
> -----------------------------------------------------------
> 
> (Updated 2009-07-22 23:57:29)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> -------
> 
> I want to add some missing functionality to the taskmanager library on the windows platform. As the code has some more X11 dependencies, I decided to make up two files for tasks.cpp which contain the functions that differ on Windows and X11.
> The only thing that should affect X11-builds in the end should be the change from class to struct for QUuid.
> There should be no other changes regarding the X11 platform.
> 
> 
> Diffs
> -----
> 
>   trunk/KDE/kdebase/workspace/libs/CMakeLists.txt 1000690 
>   trunk/KDE/kdebase/workspace/libs/taskmanager/task.h 1000690 
>   trunk/KDE/kdebase/workspace/libs/taskmanager/task.cpp 1000690 
>   trunk/KDE/kdebase/workspace/libs/taskmanager/task_win.cpp PRE-CREATION 
>   trunk/KDE/kdebase/workspace/libs/taskmanager/task_x11.cpp PRE-CREATION 
>   trunk/KDE/kdebase/workspace/libs/taskmanager/taskmanager.h 1000690 
>   trunk/KDE/kdebase/workspace/libs/taskmanager/taskmanager.cpp 1000690 
> 
> Diff: http://reviewboard.kde.org/r/1101/diff
> 
> 
> Testing
> -------
> 
> I tested this on windows with both gcc and msvc compilers.
> 
> 
> Thanks,
> 
> Patrick
> 
>