Review Request: Multiple action support for KRunner

[Aaron J. Seigo]

On Sunday 21 September 2008, Ryan P. Bitanga wrote:
> 2008/9/20 Aaron J. Seigo <aseigo at kde.org>:
> > On Thursday 18 September 2008, you wrote:
> >> > On 2008-09-17 08:34:10, Aaron Seigo wrote:
> >> 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.
>
> Deleting the action automatically removes it from the pool, so instead
> of storing the pointer we can store a QString which would be used to
> retrieve the action from the action pool. That way we could get rid of
> possible dangling pointers in QueryMatch but I don't think anyone
> would be foolish enough to delete the pointers anyway.

never underestimate people's ability to be ingeniously foolish ;) besides, 
tempting fate when we don't need to isn't smart.

> I still think a
> centralized location makes it easier to write runners by reducing
> unnecessary duplicated code per runner

not at all. we can easily make it so that runners maintain a list of their own 
actions. whatever code would end up in each runner we simple move up into 
AbstractRunner.

> (and it also allows standard actions).

as i keep saying: this is possible using other methods =) 

all the standard actions i can think of belong to the SearchContext, and 
standard actions should not be runner specific. if they are runner specific, the 
runner should simply provide them. this allows us to ignore the lifespan of a 
runner as well (Think of when the user goes and turns off a runner in the 
config, for at trivial example)

> >> 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?
>
> Yep it could.

or SearchContext even, now that i think about it some more.

> That's why the parent of QueryAction doesn't have a
> default value of 0 nor is the parent an AbstractRunner*. Since action
> providers need not be runners, other classes can be written to provide
> necessary actions and handle their execution.

how would these be different from runners? or rather, what would the API for 
these look like? (in sketch form, nothing final or overly formal/correct)

> >> 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 ...
>
> Keeping it thread-safe shouldn't make you nervous :) I only mentioned
> this to point out that it can now be made non-thread-safe.

no, it was the use of the phrase "supposed to" ;)

> >> 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.
>
> Well I think it's pretty obvious I want a three pane interface ala
> Quicksilver. :) And with plain QActions, it's difficult to determine
> whether an action has an implied target or not. I know you said that
> whether or not an action requires a target is something that could be
> resolved after the action is triggered, but the thing is there is no
> guarantee that the action _will_ be triggered.

my assumption was that actions that aren't appropriate to a match won't be 
associated with that match.

instead of throwing random actions into a pool and then sorting out afterwards 
which ones make sense, let the individual runners which actualy know which 
actions go where do this associating.

> See my window manager runner as an example.

or you could just explain the use case here instead of making everyone go read 
your code, figure out what you were trying to do and assemble the use case 
themselves. please just share what you're trying to do versus make us all go 
out and work for it.

in the window manager runner case, those actions could be created once and 
associated with each the match once the match is selected (whatever selected 
means).

what benefit is there in having all actions associated with all matches prior 
to those actions being used? none. simply ask for actions associated with a 
match when the match is selected; in the krunner UI that'd be when a context 
menu is requested; in quicksand that'd be when the third pane is populated.

> Triggering the action is left up to the runner
> author.

yes ...

> For actions that aren't owned by the runner (which can be
> determined by examining the parent), the runner can trigger the
> action.

this means coupling runners to each other such that runner A's actions and/or 
matches always mean what runner B thinks they mean. that's asking for a 
tangled web, and there must be ways to fullill whatever the use case you have 
in mind here without this kind of mess.

-- 
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/20080922/920c24d4/attachment.sig