Review Request: Multiple action support for KRunner

[Aaron J. Seigo]

On Thursday 18 September 2008, you wrote:
> > On 2008-09-17 08:34:10, Aaron Seigo wrote:
> > > 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?)
>
> Well, QueryActionPool provides an easy means to determine whether or not an
> action was created and loaded already without having to have each runner
> have its own way of tracking.

.. the downside is that now we have to coordinate the actions at a central 
point. as they are delete'able pointers and not references, that makes me 
nervous.

> It also provides a repository for shared
> actions (standard actions) common to all runners if they need any.

provided by the host application? if so, couldn't this go directly into 
RunnerManager instead?

> The
> patch originally contained a "set as default" action that could be used to
> boost the relevance of a particular match to set it as the default. It had
> a QHash<QString, QString> that contained the query and the id of the match
> but the matching had to be done in RunnerContext. That would provide us our
> first limited learning implementation in KRunner. I decided to leave it out
> to have a more focused discussion first. :)

i think this is a good idea, but one that belongs in RunnerManager rather than 
another class.

> QueryActionPool is thread-safe because it used to be accessed in the match
> methods. 

that much i understood =)

> With my latest patch, it is _supposed_ to only be accessed in the
> GUI thread.

this gets back to my nervousness ...

> I considered changing it to inherit KActionCollection, but I
> think KActionCollection is a bit too bloated and I don't like the idea of
> KActionCollection owning the actions. 

yes, i think that was a good call

> Anyone can delete the action by
> accident using KActionCollection leaving dangling pointers behind. I think
> the actions should be owned by the runners or the class in which it is
> created.

sure..

> Another possibility for QueryActionPool is something like an
> actionsForMimetype method.

wouldn't this be runner specific? if not, it also belongs in RunnerManager

> Having our own QueryAction class will allow us to put a mimetype method, a
> pointer or reference to the context, and other pertinent methods if needed.

until we have concrete use cases we don't need QueryAction, though. QAction 
would suffice until then. and we may find we never want or need anything more.

as for a mimetype method, i have a feeling that that would eventually end up 
being used to populate a ranked and/or hashed set for preferential and quick 
access .. making putting it in QueryAction itself not so useful.

-- 
Aaron J. Seigo
humru othro a kohnu se
GPG Fingerprint: 8B8B 2209 0C6F 7C47 B1EA  EE75 D6B7 2EB1 A7F1 DB43

KDE core developer sponsored by Trolltech

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: This is a digitally signed message part.
Url : http://mail.kde.org/pipermail/plasma-devel/attachments/20080919/183bf7c8/attachment-0001.sig