Merge lp://qastaging/~suutari-olli/openlp/combined-bible-quick-search into lp://qastaging/openlp

Please don't include the changed resources.py in the merge request, it makes it impossible to see the code that has actually been changed.
Places the new image in "resources/images/" if it is not already there, but during the test/review process don't include the updated resources.py. We'll generate it locally for testing.

Far to complex to translate and britle needs to be refactored to something simpler.

Not even bothered to look at. Needs to be fixed as last comment was ignored
which is not acceptable
On 4 Apr 2016 2:14 a.m., "Azaziah" <email address hidden> wrote:

> Azaziah has proposed merging
> lp:~suutari-olli/openlp/combined-bible-quick-search into lp:openlp.
>
> Requested reviews:
> Tim Bentley (trb143)
> Tomas Groth (tomasgroth)
>
> For more details, see:
>
> https://code.launchpad.net/~suutari-olli/openlp/combined-bible-quick-search/+merge/290846
>
> Thank you for your review Tim,
>
> I’ve managed to simplify the code structure.
>
> “This needs to be broken down into vers small bits which cannot be broken.
> Look at the about ui for a example.”
> I’m not sure if I understand what you meant with that.
> The example verses need to be translatable.
>
> I agree with you, the string with the scripture reference error is
> already quite horrifying and I can remember how I used to struggle
> with it for the Finnish translation. However, I feel like the
> sample verses make the message much easier to understand.
>
> If we can’t find an alternative solution for this, I propose the following:
> We (I) copy paste the old translation from scripture reference error and
> add localized sample verses by ourselves, thus limiting the possibility of
> translation screw ups. By having the template and just replacing the
> foreign Bible book names to it, it shouldn’t take too long.
>
> Refactored the code for combined search.
> - Added: def on_quick_reference_search(self):
> and moved definition of reference search there.
> - Added: def on_quick_text_search(self):
> and moved definition of text search there.
> - Removed some un-needed code duplicates
> (Double finalizing, 3rd normalizing of mouse cursor)
> - Searching scripture ref with shorter than 3 char search is now possible
> (G1 = Genesis 1)
>
> Also removed “Search” from “Search Text or Reference…”
> since it does not fit the box properly.
>
> "- Noticed I had left an old comment to a wrong place. (Moved it to def
> on_quick_reference_search)"
>
> lp:~suutari-olli/openlp/combined-bible-quick-search (revision 2624)
> [←[1;32mSUCCESS←[1;m] https://ci.openlp.io/job/Branch-01-Pull/1382/
> [←[1;32mSUCCESS←[1;m]
> https://ci.openlp.io/job/Branch-02-Functional-Tests/1300/
> [←[1;32mSUCCESS←[1;m]
> https://ci.openlp.io/job/Branch-03-Interface-Tests/1239/
> [←[1;32mSUCCESS←[1;m]
> https://ci.openlp.io/job/Branch-04a-Windows_Functional_Tests/1071/
> [←[1;32mSUCCESS←[1;m]
> https://ci.openlp.io/job/Branch-04b-Windows_Interface_Tests/662/
> [←[1;32mSUCCESS←[1;m]
> https://ci.openlp.io/job/Branch-05a-Code_Analysis/729/
> [←[1;32mSUCCESS←[1;m]
> https://ci.openlp.io/job/Branch-05b-Test_Coverage/597/
>
> ------------------------------------------------------------------------------
> This branch introduces following improvements to Quick Bible search:
> - Combined Reference/Text search which first performs the Reference
> search and then moves to Text search if nothing is found.
> - Possibility to use “.” when shortening Book names in Reference search.
> For an example Gen. 1 = Gen 1 = Genesis 1.
> - New/Improved error messages (E.g. added actual example verses
> to Reference error)
> (...

Look at about.py
Break the string in to small chunks which can be translated and then glue the lot together.

Simple 1 and 2 word phrases can be translated 10 lines of complex structure where there is no clue what needs to be done is not correct.

Also reply to comments in the comment section not the to section it gets missed.

Comments in line.
Mainly style and missing bits like comments.

Some tests will be needed.

Some of the comments are based on moved code but it was wrong then and can be made better.

Left you a challenge at the end.

I fixed most of these things and replied to the comments.
Now re-proposing. (Still need some tests though)

------------------

Comments in line.
Mainly style and missing bits like comments.

Some tests will be needed.

Some of the comments are based on moved code but it was wrong then and can be made better.

Left you a challenge at the end.

Lots of extra comments in the code which are not needed and make reading more difficult.

The search as type code needs more thinking as the timer has issues.
Need to use python 3 string change features.

Please do not keep updating the main section in each merge request use the comments. The top section is for the overall request not each individual one and that is what is merged and preserved.

See comments from previous merge and email as lost in x post

I added your comments and replied to them.

Thanks.

Not convinced with the timer. This needs thinking about.
A singleshot will only fire once.

> Not convinced with the timer. This needs thinking about.
> A singleshot will only fire once.

What do you mean "A singleshot will only fire once."?

singleshot resets timer to 0 every time a char is added or removed.
If this timer reaches 1.2 seconds, search is performed.

Timer is a must, OpenLP can't handle the fully automatic Bible text search without it.

If timer is not used, typing few chars into text search will break OpenLP
since every char triggers a new search.

Some other timer methods result in infinite loops and crash OpenLP.

> Not convinced with the timer. This needs thinking about.
> A singleshot will only fire once.

I noticed the timer had some performance issues,
I created a new timer by using a different method.

See my inline comments.

Also, please stop using settings for once-off flags. This is not a web app, it maintains state. If you can't save a flag on one object, find another shared object to use. The settings is not the right place for it.

Olli, we're using the Oxygen icon set. Can you find an appropriate icon from that set?

Olli, we're using the Oxygen icon set. Can you find an appropriate icon from that set?

I didn't know, thank you for the information!

I checked out these icons quickly but nothing really fancied my eye.
http://www.iconarchive.com/show/oxygen-icons-by-oxygen-icons.org.18.html

The combined quick search icon is self made, specifically for the purpose.
(It's kind of combining Text & Reference search icons)

For the the "Clear search results" button I searched for a sweeping broom,
which I think clearly indicates clearing of the search results.

I found it from here:
http://www.1001freedownloads.com/free-clipart/tango-edit-clear (CC0 1.0)

Is this a problem?

Somewhat, yes, because neither of them fits in with the theme. The Oxygen set is not just some random icon theme we picked up, it was the icon theme specifically created for KDE4 (where OpenLP started) and it's a cohesive set of icons.

I'll look for some icons.

Also, I don't think you added them to bzr.