-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.vidsolbach.de/r/178/#review247
-----------------------------------------------------------

Ship it!


i'm generally ok with this patch; i don't like QueryAction however. it's just 
one more class with no real purpose at this time. just use QAction's and this 
can go in.

my only other concern is having looked at the examples you've written in 
playground, none of those runners will work with the default krunner UI as the 
matches provide no default behaviour. the actions *must* be additional things 
that can be done using the match while the match itself must remain useful on 
its own.

- Aaron


On 2008-11-03 03:39:51, Ryan Bitanga wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.vidsolbach.de/r/178/
> -----------------------------------------------------------
> 
> (Updated 2008-11-03 03:39:51)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> -------
> 
> This introduces multiple action (or noun-verb) support for runners. Each 
> QueryMatch (the noun) can now have multiple actions associated with it.
> 
> Actions are QueryActions which inherit QAction. I intend to extend 
> QueryAction to support mimetypes and more functionality. If QueryActions have 
> associated mimetypes, the runner can intelligently select which of the 
> available actions to add to the match.
> 
> Another possibility would be to add object support to an action. For example, 
> a match returned by the Desktop Search runner could have the action "Copy 
> to...". The object of the action would be a directory. I'm still looking for 
> ways to implement this. One way would be to have a set of core runners 
> designated as object providers (e.g. Places, Contacts) and run a secondary 
> query on a specific runner.
> 
> Because QueryActions are GUI objects, they need to live in the GUI thread. 
> That means that QueryActions must be created outside of the match method, 
> preferably at the time of runner construction. After creation, QueryActions 
> must be registered with QueryActionPool, the global action registry. This 
> allows QueryActions to only be created once during the lifetime of a runner. 
> This also allows runners to add actions that belong to different runners. 
> However, correct execution of the action is left up to the runner in the exec 
> method. I'll provide an example of how to do this soon.
> 
> Multiple action support is optional. Runners that do not create actions will 
> continue to function just as they did prior to the introduction of multiple 
> action support.
> 
> An example implementation with multiple action support is the amarok-based 
> mediaplayer runner currently in playground. To access the actions, right 
> click on the icon and select the desired action. Note, the temporary solution 
> triggers the action upon selection the action in the context menu. Pressing 
> enter or clicking on the icon will cause the action to be performed again.
> 
> QueryAction exists to allow us to determine if a particular action requires a 
> target or not. We can't do this with plain QActions because there is no 
> guarantee that the action will be triggered. Having our own QAction 
> derivative allows us to have the interface have a way to determine if the 
> action requires a target or not and take the appropriate action. For example 
> if the action needs a directory, the interface can query a specified set of 
> runners for matches and provide these to the user. Runners are not aware of 
> the existence of the interface so there is no currently no clear way for a 
> runner to signify that a particular action requires a target. There is no 
> code as of yet that implements this since how this is going to be handled 
> needs to be discussed more thoroughly first. If ever support for actions with 
> objects is not included, QActions would suffice. However, this makes it more 
> difficult to have a compatible path for the introduction of new features in 
> the future.
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdebase/workspace/libs/plasma/CMakeLists.txt
>   /trunk/KDE/kdebase/workspace/libs/plasma/abstractrunner.h
>   /trunk/KDE/kdebase/workspace/libs/plasma/abstractrunner.cpp
>   /trunk/KDE/kdebase/workspace/libs/plasma/includes/QueryAction
>   /trunk/KDE/kdebase/workspace/libs/plasma/queryaction.h
>   /trunk/KDE/kdebase/workspace/libs/plasma/queryaction.cpp
>   /trunk/KDE/kdebase/workspace/libs/plasma/querymatch.h
>   /trunk/KDE/kdebase/workspace/libs/plasma/querymatch.cpp
>   /trunk/KDE/kdebase/workspace/libs/plasma/runnermanager.h
>   /trunk/KDE/kdebase/workspace/libs/plasma/runnermanager.cpp
> 
> Diff: http://reviewboard.vidsolbach.de/r/178/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Ryan
> 
>

_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Reply via email to