Homerun and single-runner runners

[Aaron J. Seigo]

On Wednesday, February 13, 2013 11:15:58 Aurélien Gâteau wrote:
> This means that if I add the "Activities" runner, whose default syntax is
> "activity :q:", then by default Homerun queries the runner for "activity".
> If the user types "work" in the search field, Homerun queries the runner
> for "activity work".

makes sense :)

this was one of the use cases we hoped to implement when RunnerSyntax was 
added; glad to see it may actually happen!

> 2. The way single-runners are queried is not very elegant. It looks like
> this:

this probably should be provided by RunnerManager and RunnerSyntax. sth like:

QString RunnerSyntax::asQuery(const QString &userInput = QString()) const;
bool RunnerSyntax::hasPlaceholder() const;

then there is the issue of AbstractRunners with more than one syntax .. which 
one to use? in your code the first one is used, and that will be "correct" in 
most cases .. but not when there are multiple search terms.

looking at the current runners that provide multiple search terms, it is 
apparent that:

* having more than one syntax does not mean they are all useful in single mode
* the first syntax is not always the most useful one
* multiple syntaxes may in theory be useful to be run as default, though it 
seems no runner actually has such a set of syntaxes from what i can see

we could extend RunnerSyntax so that it has a SyntaxType flag, e.g.:

	{ NoSyntaxType = 0, DefaultSyntax = 1, AutoQuerySyntax = 2}

setting DefaultSyntax would flag that syntax to be used in single runner mode, 
no query. AutoQuerySyntax would be used in single runner mode + query. that 
ought to cover all use cases?
	
and then RunnerManager::launchQuery could take all this into consideration, 
which would give this feature "for free" to all users of runners

thoughts?

that's all libplasma2 stuff, though. for now, probably "rolling it yourself" in 
the model is the only way for 4.x.
 
> QString SingleRunnerModel::prepareSearchTerm(const QString &term)
> {
>     const char *placeHolder = ":q:";
>
>     Q_ASSERT(m_manager);
>     Plasma::RunnerSyntax *syntax =
> 				m_manager->singleModeRunner()->defaultSyntax();

perhaps another assert here on m_manager->singleModeRunner()?

>     QStringList queries = syntax->exampleQueries();
>     Q_ASSERT(!queries.isEmpty());
>     QString query = queries.first();

the assert will not stop a crash in production builds.

>     if (query.contains(placeHolder)) {
>         return query.replace(placeHolder, term);
>     } else {
>         return query + ' ' + term;

this *should* fail in most cases as if the query is allowed to be followed by 
a term is *should* have a :q: there .. which means that if there is no :q: 
then there is not supposed to be a term :) the proper thing here is to just 
return the term.

for example: this won't work with the places runner as it would result in 
"places <term>" when it should just be "<term>"

>     }
> }

> 3. I noticed some runners advertise single-runner mode but do not define a
> default syntax. Is it a bug or is there a reason for that?

depends on the runner. if there is a sensible set of results to return without 
query, then there should be a default syntax. if there is not, then there 
shouldn't be one.

-- 
Aaron J. Seigo
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part.
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20130213/e08e3d21/attachment-0001.sig>