fix(search): Fix baloo searchString parsing
ClosedPublic

Authored by iasensio on Mon, Nov 11, 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
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
iasensio created this revision.Mon, Nov 11, 1:35 PM
Restricted Application added a project: Dolphin. · View Herald TranscriptMon, Nov 11, 1:35 PM
Restricted Application added a subscriber: kfm-devel. · View Herald Transcript
iasensio requested review of this revision.Mon, Nov 11, 1:35 PM
iasensio edited the test plan for this revision. (Show Details)Wed, Nov 13, 1:38 AM
bruns added inline comments.Fri, Nov 15, 9:12 PM
src/search/dolphinsearchbox.cpp
525 ↗(On Diff #69666)

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.Fri, Nov 15, 9:12 PM
This revision now requires changes to proceed.Fri, Nov 15, 9:12 PM
iasensio added inline comments.Fri, Nov 15, 9:30 PM
src/search/dolphinsearchbox.cpp
525 ↗(On Diff #69666)

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.Fri, Nov 15, 9:44 PM
src/search/dolphinsearchbox.cpp
525 ↗(On Diff #69666)

That would be wrong.

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

iasensio added inline comments.Fri, Nov 15, 10:12 PM
src/search/dolphinsearchbox.cpp
525 ↗(On Diff #69666)

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.Fri, Nov 15, 11:35 PM

Split out filename/content stuff

iasensio edited the summary of this revision. (Show Details)Fri, Nov 15, 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)Thu, Nov 28, 8:58 PM
This revision was not accepted when it landed; it landed in state Needs Review.Thu, Nov 28, 9:11 PM
This revision was automatically updated to reflect the committed changes.