'Re: Review Request: Multiple action support for KRunner'

[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-panel-devel
Subject:    Re: Review Request: Multiple action support for KRunner
From:       "Aaron Seigo" <aseigo () kde ! org>
Date:       2008-09-17 15:34:04
Message-ID: 20080917153404.18774.83637 () vidsolbach ! de
[Download RAW message or body]

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

i mostly like this patch ...

however, QueryActionPool doesn't seem to actually be used; where it is used, it seems \
to be an implementation detail and therefore shouldn't need to be exported .. but \
even more evident is that it probably isn't needed at all?

what plans do you have for QueryActionPool, if any?

(without QueryActionPool, QueryAction seems unecessary as well?)

/trunk/KDE/kdebase/workspace/krunner/resultitem.cpp
<http://reviewboard.vidsolbach.de/r/178/#comment166>

    setting a title on a QMenu doesn't actually do anything unless the menu is put \
into a submenu; KMenu lets you put a visible title in a menu, if that's what you're \
looking for.  
    if the menu title is visible in the menu itself (as opposed to a sub-menu entry \
that shows the menu), it should contain the name of the action. i18n("Actions For \
'%1'", name()) or something like that ...

/trunk/KDE/kdebase/workspace/libs/plasma/queryactionpool.h
<http://reviewboard.vidsolbach.de/r/178/#comment167>

    for applications that use runners internally, versus an application like krunner \
which is only about runners, the constructor will be used (think of multiple kickoff \
applets, for instance?); perhaps the apidox should be altered to make self() sound \
less required =)

/trunk/KDE/kdebase/workspace/libs/plasma/querymatch.h
<http://reviewboard.vidsolbach.de/r/178/#comment168>

    "current" sounds like an iterator; perhaps "selected" or "active"? \
setSelectedAction sounds good to me ..

- Aaron

On 2008-09-17 02:11:27, Ryan Bitanga wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.vidsolbach.de/r/178/
> -----------------------------------------------------------
> 
> (Updated 2008-09-17 02:11:27)
> 
> 
> 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. 
> As of the latest patch, registration and unregistration from the global action \
> registry is handled by the QueryAction itself. Actions are added by the \
> actionsForMatch() method in abstract runner allowing on-demand creation of actions \
> if so desired. 
> 
> Diffs
> -----
> 
> /trunk/KDE/kdebase/workspace/krunner/resultitem.h
> /trunk/KDE/kdebase/workspace/krunner/resultitem.cpp
> /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/includes/QueryActionPool
> /trunk/KDE/kdebase/workspace/libs/plasma/queryaction.h
> /trunk/KDE/kdebase/workspace/libs/plasma/queryaction.cpp
> /trunk/KDE/kdebase/workspace/libs/plasma/queryactionpool.h
> /trunk/KDE/kdebase/workspace/libs/plasma/queryactionpool.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

[prev in list] [next in list] [prev in thread] [next in thread] 