Review Request: Window runner to switch windows and desktops

[Aaron Seigo]

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


ok, now the implementation review part of things :)

one thing that is useful to observe about this is that it takes a more memory intensive approach, storing the individual icons, window info and winid of all available windows, to make matching (at least in theory?) faster.

the downside is that krunner is going to wake up (and therefore stay in main memory and be executing code) whenever windows change. that may not be exactly optimal ;)

the windowinfo isn't actually used except for when one of the keywords like desktop is used, and the icons aren't used unless a match is made. but fetching the list of windowinfo in each pass through match can't be great either.

what would really be useful here would be some way to tell the runner "you should be prepping yourself for use now" and then later "ok, we're done with this search session". krunner itself could delete the runners between invocations of the interface, but depending on the runner that can be rather slow-ish and if there are runners that ever require a constant and stateful connection to something external that would hurt.

so it would seem we need some mechanism to prod a runner to get ready for searches and then let it know again when it can not worry about it at all. that way this runner could populate the windowInfo collection and only pay attention to window changes when the krunner interface is actually activated by the user.



trunk/KDE/kdebase/workspace/plasma/runners/windows/windowsrunner.cpp
<http://reviewboard.kde.org/r/1114/#comment1168>

    plasma style is " } else if (..) {"
    
    if this is meant to go into kdebase (where i thin it does belong) those ws changes will need to be made at some point :)



trunk/KDE/kdebase/workspace/plasma/runners/windows/windowsrunner.cpp
<http://reviewboard.kde.org/r/1114/#comment1167>

    don't use keys(); it's a very slow mechanism (iterates over the collection, allocates a new list, and then you iterate over that again in the foreach). should use a QMapIterator here instead



trunk/KDE/kdebase/workspace/plasma/runners/windows/windowsrunner.cpp
<http://reviewboard.kde.org/r/1114/#comment1169>

    should use a proper iterator here rather than keys()



trunk/KDE/kdebase/workspace/plasma/runners/windows/windowsrunner.cpp
<http://reviewboard.kde.org/r/1114/#comment1170>

    another usage of keys()


- Aaron


On 2009-07-26 12:34:22, Martin Gräßlin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/1114/
> -----------------------------------------------------------
> 
> (Updated 2009-07-26 12:34:22)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> -------
> 
> This runner lists the windows and virtual desktops. It allows to activate a selected window or switch to a selected desktop. The runner works in the following way:
>  * input is matched on window name, class or role; matching windows are listed
>  * input is matched on desktop name. Matching desktops are shown for switching to, all windows on matching desktops are listed.
>  * keyword "desktop": lists all desktops (except current) if additional number is inserted the list is reduced to that desktop
>  * keyword "window": lists all windows. Additional text will be used to restrict like in first case. Following sub queries to restrict search are possible:
>    * name=: restrict on name
>    * class=: restrict on window class
>    * role=: restrict on window role
>    * desktop=: restrict on desktop
>   those subqueries can be combined in any order. Each input not containing a '=' will be interpreted as a name restriction if there is no explicit name restriction. In that case it's ignored. Example query: "window desktop=2 class=kmail role=composer" will list all open KMail composer windows on desktop 2.
> 
> 
> Diffs
> -----
> 
>   trunk/KDE/kdebase/workspace/plasma/runners/CMakeLists.txt 1000707 
>   trunk/KDE/kdebase/workspace/plasma/runners/windows/CMakeLists.txt PRE-CREATION 
>   trunk/KDE/kdebase/workspace/plasma/runners/windows/plasma-runner-windows.desktop PRE-CREATION 
>   trunk/KDE/kdebase/workspace/plasma/runners/windows/windowsrunner.h PRE-CREATION 
>   trunk/KDE/kdebase/workspace/plasma/runners/windows/windowsrunner.cpp PRE-CREATION 
> 
> Diff: http://reviewboard.kde.org/r/1114/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Martin
> 
>