fix(search): Fix baloo searchString parsing
ClosedPublic

Authored by iasensio on Nov 11 2019, 1:35 PM.

Details

Summary

Fix the parsing of Baloo query searchString to represent its parameters properly
in the search box:

  1. Baloo terms (rating, modified) are added to the user search text:
  2. Extra quotes are added to the search text: https://bugs.kde.org/show_bug.cgi?id=412952

This revision supersedes D24422, by making the fixes on the new dolphin query model,
instead of directly on the UI.

BUG: 412952
FIXED IN: 19.11.90

Test Plan
  • bin/dolphinquerytest passes without XFAILs
  • Dolphin search box is not garbled by search terms or quotes

Diff Detail

Repository
R318 Dolphin
Branch
fix_baloo_parsing
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 18741
Build 18759: arc lint + arc unit
iasensio created this revision.Nov 11 2019, 1:35 PM
Restricted Application added a project: Dolphin. ยท View Herald TranscriptNov 11 2019, 1:35 PM
Restricted Application added a subscriber: kfm-devel. ยท View Herald Transcript
iasensio requested review of this revision.Nov 11 2019, 1:35 PM
iasensio edited the test plan for this revision. (Show Details)Nov 13 2019, 1:38 AM
bruns added inline comments.Nov 15 2019, 9:12 PM
src/search/dolphinsearchbox.cpp
525

This is conceptually wrong.

You can use e.g. all files with a filename matching "Report" and having e.g. "Foo" in the full text.

bruns requested changes to this revision.Nov 15 2019, 9:12 PM
This revision now requires changes to proceed.Nov 15 2019, 9:12 PM
iasensio added inline comments.Nov 15 2019, 9:30 PM
src/search/dolphinsearchbox.cpp
525

From a custom-made search url or baloosearch that is true, but if the search is done within Dolphin's UI, the full input text will be either used as filename or as content.

bruns added inline comments.Nov 15 2019, 9:44 PM
src/search/dolphinsearchbox.cpp
525

That would be wrong.

The parser should not break what has been typed and interfere with the user.

iasensio added inline comments.Nov 15 2019, 10:12 PM
src/search/dolphinsearchbox.cpp
525

I've given it a thought and to do it properly it will require some changes in the model, and a new test case would be handful. If you'd prefer I can split the content/filename part (case 3.) from the current patch, both in model and ui, and come with a better solution in a later one.

iasensio updated this revision to Diff 69827.Nov 15 2019, 11:35 PM

Split out filename/content stuff

iasensio edited the summary of this revision. (Show Details)Nov 15 2019, 11:38 PM
iasensio edited the test plan for this revision. (Show Details)

Any more thoughts on this? I wouldn't like to miss this fix for release/19.12 ๐Ÿ˜ƒ

@bruns Ping.
We have until November 28 to get this in.

@elvisangelaccio, @bruns, if there is no strong objections, I would like to push this fix to release/19.12 today, before the RC tagging

@elvisangelaccio, @bruns, if there is no strong objections, I would like to push this fix to release/19.12 today, before the RC tagging

Yes please. Thanks!

iasensio edited the summary of this revision. (Show Details)Nov 28 2019, 8:58 PM
This revision was not accepted when it landed; it landed in state Needs Review.Nov 28 2019, 9:11 PM
This revision was automatically updated to reflect the committed changes.