Implement lazy search #581
Conversation
tobiolo
force-pushed
the
lazy-search
branch
6 times, most recently
from
0af179c to
bf0bfd1
Compare
src/document.h
Outdated
| @@ -2024,10 +2023,21 @@ struct Document { | |||
| } | |||
| } | |||
|
|
|||
| const wxChar *LoadSearch() { | |||
There was a problem hiding this comment.
Unless they're gigantic, I'd generally say there is no good reason introducing functions that only get called once, makes the code harder to follow
There was a problem hiding this comment.
Thanks for your feedback.
Ok, I changed that.
src/document.h
Outdated
| if (!c->text.t.Len()) return _(L"No text in this cell."); | ||
| sys->frame->filter->SetFocus(); | ||
| sys->frame->filter->SetValue(c->text.t); | ||
| return _(L"Search string was loaded from selection."); |
There was a problem hiding this comment.
This status is maybe superfluous, status should only be set on special events
There was a problem hiding this comment.
This status is maybe superfluous, status should only be set on special events
Ok, I was unsure myself. I replaced it with nullptr.
src/document.h
Outdated
| return SearchNext(dc, false, true, true); | ||
| return sys->searchstring.Len() | ||
| ? SearchNext(dc, false, true, k == A_SEARCHPREV) | ||
| : LoadSearch(); |
There was a problem hiding this comment.
this function only sets the search, so does that mean I have to press search twice to do the actual search? what is the idea behind that UI (as opposed to searching directly?)
There was a problem hiding this comment.
The main idea is to implement some lazy search option if one wants to look for the same text in other cells without having to copy & paste the cell text to the filter text control.
I think of two reasons for having to do it twice (load the cell text into the filter text control and actually do the search with SearchNext):
- First provide some kind of visual feedback that the search is now active (the cells are darkened) and ready to proceed. Something of the kind: "The search term is loaded". The darkening also provides some kind of visual orientation (are matches nearby?).
- The search parameters are set by
MyFrame::OnSearchthat is run when the value of thewxTextCtrlfilteris changed. This is triggered bySetValue(...). SoOnSearchruns event-driven/asynchronously and we cannot just runSearchNextwithout knowing for sure that the search parameters are already all set (thus avoiding a race).
There was a problem hiding this comment.
Ok, I suppose that's fine for the moment.. much better than not working at all :)
There was a problem hiding this comment.
Thanks for your review!
If no search term is given and one cell is selected and the user triggers the search, use the cell text string as search query. This is similiar to the F6 and F7 (jump to text/image) experience. Also make the rootgrid check the first check for futureproofness.
If no search term is given and one cell is selected and the user triggers the search, use the cell text string as search query.
This is similiar to the F6 and F7 (jump to text/image) experience.
Also make the rootgrid check the first check for futureproofness.