Homerun sources and Plasma runners

[Aurélien Gâteau]

Le samedi 17 novembre 2012 12:56:13 Aaron J. Seigo a écrit :
> On Friday, November 16, 2012 17:04:04 Aurélien Gâteau wrote:
> > A preliminary subject is API compatibility: I don't think it is possible
> > to extend the current AbstractRunner interface in a BC way to meet our
> > needs. Would you be ready to create a new runner interface?
> > 
> > This would require:
> > - Defining and implementing this new interface (let's call it
> > 
> >    AbstractRunner2 for now)
> > 
> > - Implementing an adapter to load old AbstractRunner-based runners
> > - Porting KRunner to use AbstractRunner2 and the adapter
> > - Progressively porting runners shipped in KDE SC to AbstractRunner2
> 
> this should be unnecessary. first, we can look at what API is missing or
> needs adjusting.
> 
> assuming changes are needed, we can prototype this in homerun (and so
> homerun can benefit from the improvements sooner) with an AbstratRunner
> subclass there which runners built for homerun can utilize. this API would
> then go directly into libplasma2 and Plasma Desktop 2 would include a
> krunner that uses this new API.

Agreed.

> > Sources features missing in Runners:
> > - model-based => more QML friendly
> 
> as with Marco, i don't see any real need to have this in the libplasma API,
> but if we do want this in the libplasma API, we don't need to expose it from
> AbstractRunner ..

Just answered Marco on this point.

> RunnerManager could provide access to a model. in libplasma2 it could even
> become a model itself. right now, the runner model in kde-runtime is
> essentially exactly that -> it provides access to setting the query (just
> forwards that call to its internal RunnerManager), etc. so RunnerManager
> could simply become a model libplasma2 .. and in libplasma1 we can continue
> to use the model-on-top-of-RunnerManager.

Makes sense.
> 
> so .. question is how best to implement that kind of model and that brings
> us
> to the next point:
> > - can update matches
> 
> matches have id()s which can be used to reference them. what would be needed
> is for RunnerContext (which colates the matches from all the runners via
> addMatches) to check the id of matches and replace them.
> 
> implementation detail: this could be done using a hash to keep lookup fast.
> since id's are not guaranteed to be unique, this would need to be a
> multihash, or possibly a hash of lists.

I didn't know ids where not guaranteed to be unique. That sounds dangerous.

> then there are two possibilities:
> 
> a) RunnerContext gets a new updateMatches(const QList<QueryMatch> &matches)
> method which would add the matches and update matches with the same id.
> (implementation: it could actually just remove the old match(es) and add the
> new ones; as long as there is a model tracking the row<->match correlation
> properly, these updates could be communicated via dataChanged signals)
> 
> b) RunnerContext::addMatches treats all calls as updates, so if a runner
> calls addMatches more than once and there are matches with the same id's,
> they'd get replaced. this could be problematic, however, as it would create
> a change in behaviour for runners that call addMatches multiple times in
> their match method and don't call setData or setId on the matches
> explicitly. for this reason, i prefer (a)

I prefer (a) as well. I would actually prefer using the existing Qt model API, 
but that would not be source compatible.

> > - "standardized" favoriteIds
> 
> because it uses strings and relies on parsing of them, i really dislike this
> mechanism. it's too easy to get wrong (e.g. a runner that by chance uses
> one of the prefix strings in a match id) and makes it unecessarily
> difficult to change in future.

I don't think this could happen, favoriteIds are not the same concept as 
querymatch ids. It is a way to tell Homerun what an item is, using Homerun 
favoriteId "format" (which indeed is string-based and quite hackish).

> RunnerContext has this enum in it:
> 
>         enum Type {
>             None = 0,
>             UnknownType = 1,
>             Directory = 2,
>             File = 4,
>             NetworkLocation = 8,
>             Executable = 16,
>             ShellCommand = 32,
>             Help = 64,
>             FileSystem = Directory | File | Executable | ShellCommand
>         };
> 
> this could be used in QueryMatch as well .. and then knowing what kind of
> favourite it creates is a simple matter of checking this attribute. there is
> already a type property in QueryMatch used with the QueryMatch::Type enum.
> so perhaps:
> 
> 	void QueryMatch::setCategory(RunnerContext::Type category)
> 	RunnerContext::Type QueryMatch::category() const
> 
> this means one more int stored for each QueryMatch and using RunnerContext's
> header in QueryMatch (though in practice this is a complete non-issue since
> almost everything that touches a QueryMatch also uses RunnerContext's API
> and they are linked functionality-wise)

This sounds good to identify favorites, but it does not define how to get the 
actual data. There need to be at least a convention on how to get the url of a 
file or directory from a match (I guess from mimeDataForMatch()), same for 
application name.

>From an API point-of-view, I'd rather ignore the RunnerContext enum and define 
a MatchCategory in QueryMatch. I am actually wondering how RunnerContext::Type 
is defined and where it is used.

> > - browsing support (as in: entering/leaving folders, application
> > 
> >    groups...) by invoking the source with named arguments
> 
> this is already possible -> simply run the result of QueryMatch as a new
> query. this is close to how InformationalMatches are treated already, in
> fact.
> 
> so a runner that wants to list some directory structure could return a list
> of matches, some of which represents folders which when selected would be
> used as the new query. this would not work for viewing entire trees at
> once, unless a different RunnerContext was used for each new "sub-"query of
> the runner. however, i don't think viewing trees is a valid requirement for
> runners, so this is imho a non-issue.
> 
> to make this easier and less hacky, a new value could be added to
> QueryMatch::Type; e.g. QueryMatch. then items that can be re-used to query
> the source runner can be marked with that type. voila.

This sounds promising, but isn't it going to be a problem for KRunner? If a 
match is of type QueryMatch, what is KRunner going to do with it?
> 
> > - future need: reorder items
> 
> can you describe the needs here more clearly? because i think this is
> already possible given that sorting is done by the visualization anyways.

For the FavoritePlaces and FavoriteApps sources, we want users to be able to 
reorder items.

> something that is not covered here which would actually be very useful is
> results paging. so a runner would return at most N results (so for a set of
> M runners, the maximum number of returned matches would be N*M) but then
> could be requested to get anothe rset of N results.

This is solved by Qt models as well with QAbstractItemModel::fetchMore() and 
canFetchMore().

Aurélien