Introduce ResultsModel with better data processing and filtering
ClosedPublic

Authored by broulik on Jul 25 2019, 9:25 AM.

Details

Reviewers
davidedmundson
Group Reviewers
Plasma
Summary

This adds a new Milou.ResultsModel for runner results which tries very hard to avoid model resets and instead signals addition, removal, and changes in results. One of the main advantages is that now the highlighted item isn't reset every time the results change, i.e. results that come in delayed won't unexpectedly change the keyboard selection.

The various result processing, filtering, and grouping features are split into tiny dedicated proxy models to make the code easier to follow and maintain. The QML API isn't changed so that look and feel themes don't need to be adjusted.

The raw results model is a tree of runner results grouped by matchCategory (e.g. "Documents", "Applications").
Results are then sorted by result type (ExactMatch, HelperMatch, etc) and by their relevance. The highest scored match in a group determines its overall score. The previous heuristic of prefering categories who have results with the query in their visible text has been removed but could easily be added back if deemed beneficial for the quality of results.

The groups are then limited in size with higher scored categories being allowed to show more matches. This model could in the future also be extended to allow expanding groups to show all of the results inside.
After that the model is flattened back into a tree with the group items removed for consumption in the existing ListView. Items with an identical text will have their subtext displayed automatically for disambiguation.

Test Plan

Branch broulik/newresultsmodel in Milou

  • KRunner feels snappier overall, especially when dealing with a result set that hardly changes, e.g. using the calculator runner
  • Ran model test and fixed most of its complaints. However, when updating a category, we append the new items, replace the entire list, and afterwards signal a data change, which model test doesn't like. It checks for whether the data before the inserted area didn't change, which it did at this point but that's why we signal a data change afterwards :)
  • Running results still works
  • Replacing the query string when requested still works, e.g. clicking on a calculator runner result changes the query string to that number
  • Dragging results from KRunner to elsewhere still works. I noticed dragging applications from krunner to kickoff doesn't work but didn't check if that is a regression or because KRunner closes when kickoff opens
  • Invoking runner actions (e.g. "run in terminal") still work

Diff Detail

Repository
R112 Milou
Lint
Lint Skipped
Unit
Unit Tests Skipped
broulik created this revision.Jul 25 2019, 9:25 AM
Restricted Application added a project: Plasma. · View Herald TranscriptJul 25 2019, 9:25 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
broulik requested review of this revision.Jul 25 2019, 9:25 AM
broulik edited the test plan for this revision. (Show Details)Jul 25 2019, 9:33 AM
davidedmundson added inline comments.
lib/resultsmodel.cpp
155

Future Kai is going to appreciate some docs on what each class does.

lib/runnerresultsmodel.cpp
147–149

Urgh, we've done this style before and it resulted in obscure QtQuick crashes.

For QtQuick we need to insert new rows for new rows, not just insert/remove the difference and emit changed on everything, otherwise we mess up the delegates.

See 9371a7c96d4722b93b49b18df7b21780f2a9ad38 in p-d

Sorry.

broulik updated this revision to Diff 62540.Jul 25 2019, 2:07 PM
broulik edited the summary of this revision. (Show Details)
  • Emit dataChanged before inserting and only update the to be signalled range
  • Add some docs to make future me happy
  • Fix current index behavior with mouse and keyboard
apol added a subscriber: apol.Jul 25 2019, 2:33 PM

LGTM overall

lib/qml/ResultsView.qml
94

Call it resetView()? Otherwise it seems like you're resetting the model again.

Nearly all good

lib/qml/ResultsView.qml
90

This recreates the bug I just fixed!

Adding a Qt.callLater round this would also work

broulik updated this revision to Diff 62595.Jul 26 2019, 11:48 AM
  • Fix running results automatically
  • Fix warnings
davidedmundson accepted this revision.Jul 26 2019, 2:39 PM
This revision is now accepted and ready to land.Jul 26 2019, 2:39 PM
broulik closed this revision.Jul 26 2019, 2:41 PM

Merged into master