Create Task
Maniphest T13616

KRunner: Rethink RunnerContext::Types usage
Open, Needs TriagePublic
Actions

Description

Currently the RunnerContext::Type is used by only very few runners in the actual match logic.
But it is used for the ignore types, this way a runner can for example specify that it should not be queried if the context type is a shell command. This brings a few issues:

  • This is not available for the D-Bus runners
  • We are required to do quite a lot of lookup/parsing logic and can't be sure if it is even needed
    • The KIO library is only used for determining the context type (KProtocolInfo::classInfo)
  • The types are vague, if we were to type "spell" KRunner would think that is an executable, but it is intended as a trigger word. Consequently the runners can only exclude very few types.
    • The minimumLetterCount property which was recently introduced also allows the runners to prevent the match method from being called if the query would get rejected anyway. This is also implemented at a higher level and supports D-Bus runners.
    • A match regex will also be implemented, so that runners with specific trigger words don't get queried unnecessarily.

So to conclude: This feature is not very beneficial and far better alternatives are implemented/will be implemented.

Related Objects
Search...

alex created this task.Sep 13 2020, 8:59 PM
alex added a comment.EditedOct 8 2020, 6:04 AM

Also the value set by the AbstractRunner::setIgnoredTypes gets only once read in the RunnerManager. There it is checked if the type of the context is in the blacklisted list of types from the runner. If yes the runner does not get queried.

We already have a similar logic which for the X-Plasma-Runner-Min-Letter-Count and X-Plasma-Runner-Match-Regex properties which have landed. With the difference that the runners can be there far more precise in setting the constraints. And these two properties don't do the validation based on the type but rather just the typed text, which makes IMO far more sense.

Consequently I would argue that it would be best to deprecate this.

alex renamed this task from KRunner: Rethink match type to KRunner: Rethink RunnerContext::Types usage.Oct 8 2020, 6:05 AM
alex updated the task description. (Show Details)
alex added a subscriber: davidre.Oct 8 2020, 8:27 AM
alex added a comment.EditedOct 11 2020, 8:39 AM

This also causes https://bugs.kde.org/show_bug.cgi?id=342876, because runnercontext.cpp in line 187 does

if (correctPathCase(path, correctCasePath)) {

Which fixes the casing errors, but does not update the query string. So the type is Directory or File, but the dir/file does not exist.

Also the QString RunnerContext::mimeType() method seems completely unused.

So I would plan to refactor this class to being a simple container for the matches and some data(like query, single runner mode)

davidedmundson added a subscriber: davidedmundson.Oct 16 2020, 4:38 PM

Currently the RunnerContext::Type is used by only very few runners in the actual match logic.

Yeah, this whole aspect where runnermanager does some analysis of what it thinks you're searching for doesn't really match our usages.

alex added a comment.Dec 13 2020, 11:22 AM

The deprecation MR https://invent.kde.org/frameworks/krunner/-/merge_requests/40 and https://invent.kde.org/plasma/plasma-workspace/-/merge_requests/512 which ports the only runner which relies on the the type for the match logic.

In KF6 we can drop this and get rid of the KIO dependency.

alex moved this task from Backlog to Waiting on KF6 Branching on the KF6 board.Jan 9 2021, 8:57 AM
alex moved this task from Waiting on KF6 Branching to Done on the KF6 board.Feb 7 2023, 8:15 AM

All of this has been cleaned up with the removal of deprecated API