Review Request: Allow runners to do quick prepare and teardown for match sessions

Ryan Bitanga ryan.bitanga at gmail.com
Mon Jul 27 18:57:14 CEST 2009


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

Ship it!


I like the concept. I can't think of better names for the methods either. How about setupMatchSession() instead?


/trunk/KDE/kdelibs/plasma/runnermanager.cpp
<http://reviewboard.kde.org/r/1133/#comment1173>

    Isn't this line unnecessary? It's called in the other overloaded function.


- Ryan


On 2009-07-27 00:19:26, Aaron Seigo wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/1133/
> -----------------------------------------------------------
> 
> (Updated 2009-07-27 00:19:26)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> -------
> 
> Right now runners have only one opportunity to set themselves up: when init() is called. It has become apparent that for _some_ runners it is advantageous to do a bit of set up before a matching session starts and teardown afterwards. For instance, the new window runner needs to keep track of the windows that exist and it shouldn't be getting that information constantly as the user types; equally so, it shouldn't be holding onto and keeping this information up to date even when the krunner interface isn't shown.
> 
> This patch provides a set of signals that runners may optionally listen to. A user of RunnerManager may call prepareForMatchSession and matchSessionComplete itself, or just let RunnerManager call prepareForMatchSession itself if it doesn't care about resource usage.
> 
> Users of AbstractRunner which aren't using RunnerManager to do so can "fudge" this by connecting a local signal to the AbstractRunner signal and then emitting the local signal inside the local code. Not overly elegant, but certainly an edge case (and one we don't have right now). All AbstractRunner usage really should be going through RunnerManager.
> 
> I'm still looking for better names for the new methods/signals, so suggestions welcome there, too! Wanted to get some feedback on the design earlier, though.
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdebase/workspace/krunner/interfaces/default/interface.cpp 1002691 
>   /trunk/KDE/kdebase/workspace/krunner/krunnerdialog.h 1002202 
>   /trunk/KDE/kdebase/workspace/krunner/krunnerdialog.cpp 1002202 
>   /trunk/KDE/kdelibs/plasma/abstractrunner.h 1002205 
>   /trunk/KDE/kdelibs/plasma/runnermanager.h 1002205 
>   /trunk/KDE/kdelibs/plasma/runnermanager.cpp 1002205 
> 
> Diff: http://reviewboard.kde.org/r/1133/diff
> 
> 
> Testing
> -------
> 
> built, ran krunner ...
> 
> 
> Thanks,
> 
> Aaron
> 
>



More information about the Plasma-devel mailing list