Differential D24577

[dolphin/search] Fix search behavior when selecting "Your Files"
ClosedPublic
Actions

Authored by iasensio on Oct 11 2019, 11:50 PM.

Details

Reviewers
elvisangelaccio
Group Reviewers
Dolphin
Commits
R318:167d5184eda1: [dolphin/search] Fix search behavior when selecting "Your Files"
Summary

Fix the search box forgetting the location where the user was previously searching into.

To do a search on "All Files" instead of setting the m_searchPath to $HOME, it checks the button state, so the "From Here" location is not lost.
As an added benefit, selecting "Your Files" when in a non-indexed folder will use the baloo search instead of a fully non-indexed search from $HOME.

This issue is the last remaining one of the series started with D24422, with the purpose of fixing the searchbox parsing and update.

Test Plan
  • Toggle between "From Here/Your Files" and navigate between locations
  • The search box remembers the location and keeps a coherent state

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.Oct 11 2019, 11:50 PM
Restricted Application added a project: Dolphin. · View Herald TranscriptOct 11 2019, 11:50 PM
Restricted Application added a subscriber: kfm-devel. · View Herald Transcript
iasensio requested review of this revision.Oct 11 2019, 11:50 PM
elvisangelaccio added inline comments.Oct 13 2019, 3:51 PM
src/search/dolphinsearchbox.cpp
595

Shouldn't we replace all the other usages of m_searchPath with searchPath() ?

iasensio updated this revision to Diff 67860.Oct 13 2019, 5:57 PM
iasensio marked an inline comment as done.
  • Rebase to master
  • Fix non-indexed corner case
  • Make use of searchPath()
iasensio added a comment.Oct 13 2019, 5:57 PM

@elvisangelaccio, about your question, reviewing the code, there are four places (apart from setter and getter) where m_searchPath is used:

  1. For baloo search (l.502), it is only used if m_everywhere is not checked, so it would cancel the condition in the getter, returning m_searchPath anyway
  2. For regular search (l.135), there is a similar condition as in searchPath().
  3. Menu action to call an external tool (KFind) (l.423)
  4. In the entry point fromUrlSearch(), introduced in this patch to reset it on certain paths

I've change the point 2. to reuse searchPath() since it was already doing the same thing. In the other cases I think that it is better to keep it at is, but please tell me if you think otherwise.

elvisangelaccio accepted this revision.Oct 20 2019, 9:55 AM
In D24577#546629, @iasensio wrote:

@elvisangelaccio, about your question, reviewing the code, there are four places (apart from setter and getter) where m_searchPath is used:

  1. For baloo search (l.502), it is only used if m_everywhere is not checked, so it would cancel the condition in the getter, returning m_searchPath anyway
  2. For regular search (l.135), there is a similar condition as in searchPath().
  3. Menu action to call an external tool (KFind) (l.423)
  4. In the entry point fromUrlSearch(), introduced in this patch to reset it on certain paths

    I've change the point 2. to reuse searchPath() since it was already doing the same thing. In the other cases I think that it is better to keep it at is, but please tell me if you think otherwise.

Thanks, LGTM as-is.

This revision is now accepted and ready to land.Oct 20 2019, 9:55 AM
Closed by commit R318:167d5184eda1: [dolphin/search] Fix search behavior when selecting "Your Files" (authored by iasensio, committed by elvisangelaccio). · Explain WhyOct 20 2019, 9:55 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 68336

View Options

src/search/dolphinsearchbox.h

Loading...
View Options

src/search/dolphinsearchbox.cpp

Loading...