openlp-core team mailing list archive

[Andreas Preikschat <andreaspreikschat@xxxxxxxxxx>]

Review: Needs Fixing

Hello,

Sorry, for not providing any feedback.

> I propose that while doing searching in response to typing, rather than
> pressing search, if the limit search checkbox is checked, the last entered
> search results is used to limit the search, but that set is not updated. The
> last entered search results is updated when either: enter is pressed or search
> clicked or the checkbox is cleared and then set again.

For me it is very important, that your code (or anybodies code) is consistent. Clear rules are important (makes it easier to code, and easier to use). 

I tried this:
1) Searched for "new" in the entire song.
2) Then I changed the search "type" to "author". This seems to rest the last search result.

I would expect that the last search result list is not reset.

Line 171: Please add docstrings.

I think that "Limit search" is not clear enough. I don't have a better suggestion (sorry), maybe ask in IRC (or on the mailinglist). Also extend the tooltip. I think we should be more precious (and the tooltip text should end with a full stop).

And please merge trunk.

Keep it up :)
-- 
https://code.launchpad.net/~martin-hosken/openlp/subsearch/+merge/152404
Your team OpenLP Core is subscribed to branch lp:openlp.