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)
>   (Parts of the new messages are Bolded so <br> is required since
>    \n does not work with bolding)
>
> This branch also prevents users from performing Text searches which are:
> - Shorter than 3 characters long (not including spaces)
> - Searches consisting from only spaces
>
> These currently possible bad search quarries result in LONG search times
> and program instability/crashing. It’s still possible to search 3
> characters
> separated by spaces, but that scenario is relatively rarer.
> --
> You are requested to review the proposed merge of
> lp:~suutari-olli/openlp/combined-bible-quick-search into lp:openlp.
>
> === modified file 'openlp/core/common/uistrings.py'
> --- openlp/core/common/uistrings.py     2016-01-23 08:15:37 +0000
> +++ openlp/core/common/uistrings.py     2016-04-04 01:13:54 +0000
> @@ -151,3 +151,22 @@
>          self.Version = translate('OpenLP.Ui', 'Version')
>          self.View = translate('OpenLP.Ui', 'View')
>          self.ViewMode = translate('OpenLP.Ui', 'View Mode')
> +        self.BibleShortSearchTitle = translate('OpenLP.Ui', 'Search is
> Empty or too Short')
> +        self.BibleScriptureError = translate('OpenLP.Ui', '<br><br>Book
> Chapter | John 3:16<br>'
> +                                                          'Book
> Chapter%(range)sChapter | John 3%(range)s4<br>'
> +                                                          'Book
> Chapter%(verse)sVerse%(range)sVerse | John 3%(verse)'
> +
> 's16%(range)s17<br>Book Chapter%(verse)sVerse%(range)sVerse%'
> +
> '(list)sVerse%(range)sVerse | John 3%(verse)s16-17%(list)s20%'
> +
> '(range)s22<br>Book Chapter%(verse)sVerse%(range)sVerse%'
> +
> '(list)sChapter%(verse)sVerse%(range)sVerse | John 3%(verse)'
> +
> 's16%(range)s17%(list)s5%(verse)s7%(range)s9<br>Book Chapter%'
> +
> '(verse)sVerse%(range)sChapter%(verse)sVerse | John 3%(verse)'
> +
> 's16%(range)s4%(verse)s2<br><br> Book names may be shortened '
> +                                                          'from full
> names, for an example: Joh 3 = John 3',
> +                                             'Please pay attention to the
> appended "s" of the wildcards and refrain '
> +                                             'from translating the words
> inside the names in the brackets.')
> +        self.BibleShortSearch = translate('OpenLP.Ui', '<strong>The
> search you have entered is empty or shorter '
> +                                                       'than 3 characters
> long.<br>Please try again with '
> +                                                       'a longer
> search.</strong><br><br>You can separate different '
> +                                                       'keywords by a
> space to search for all of your keywords and you '
> +                                                       'can separate them
> by a comma to search for one of them.')
>
> === modified file 'openlp/plugins/bibles/lib/manager.py'
> --- openlp/plugins/bibles/lib/manager.py        2015-12-31 22:46:06 +0000
> +++ openlp/plugins/bibles/lib/manager.py        2016-04-04 01:13:54 +0000
> @@ -23,7 +23,7 @@
>  import logging
>  import os
>
> -from openlp.core.common import RegistryProperties, AppLocation, Settings,
> translate
> +from openlp.core.common import RegistryProperties, AppLocation, Settings,
> UiStrings, translate
>  from openlp.core.utils import delete_file
>  from openlp.plugins.bibles.lib import parse_reference,
> get_reference_separator, LanguageSelection
>  from openlp.plugins.bibles.lib.db import BibleDB, BibleMeta
> @@ -279,21 +279,10 @@
>                      'list': get_reference_separator('sep_l_display')}
>                  self.main_window.information_message(
>                      translate('BiblesPlugin.BibleManager', 'Scripture
> Reference Error'),
> -                    translate('BiblesPlugin.BibleManager', 'Your
> scripture reference is either not supported by '
> -                              'OpenLP or is invalid. Please make sure
> your reference '
> -                              'conforms to one of the following patterns
> or consult the manual:\n\n'
> -                              'Book Chapter\n'
> -                              'Book Chapter%(range)sChapter\n'
> -                              'Book Chapter%(verse)sVerse%(range)sVerse\n'
> -                              'Book
> Chapter%(verse)sVerse%(range)sVerse%(list)sVerse'
> -                              '%(range)sVerse\n'
> -                              'Book
> Chapter%(verse)sVerse%(range)sVerse%(list)sChapter'
> -                              '%(verse)sVerse%(range)sVerse\n'
> -                              'Book
> Chapter%(verse)sVerse%(range)sChapter%(verse)sVerse',
> -                              'Please pay attention to the appended "s"
> of the wildcards '
> -                              'and refrain from translating the words
> inside the names in the brackets.')
> -                    % reference_separators
> -                )
> +                    translate('BiblesPlugin.BibleManager',
> '<strong>OpenLP couldn’t find anything '
> +                                                           'with your
> search.<br><br>'
> +                              'Please make sure that your reference
> follows one of these patterns:</strong>%s'
> +                              % UiStrings().BibleScriptureError %
> reference_separators))
>              return None
>
>      def get_language_selection(self, bible):
> @@ -339,22 +328,40 @@
>          if second_bible:
>              second_web_bible =
> self.db_cache[second_bible].get_object(BibleMeta, 'download_source')
>          if web_bible or second_web_bible:
> +            self.application.set_normal_cursor()
>              self.main_window.information_message(
>                  translate('BiblesPlugin.BibleManager', 'Web Bible cannot
> be used'),
> -                translate('BiblesPlugin.BibleManager', 'Text Search is
> not available with Web Bibles.')
> +                translate('BiblesPlugin.BibleManager', 'Text Search is
> not available with Web Bibles.\n'
> +                                                       'Please use the
> Scripture Reference Search instead.')
>              )
>              return None
> -        if text:
> +        if len(text) - text.count(' ') < 3:
> +            self.main_window.information_message(
> +                ('%s' % UiStrings().BibleShortSearchTitle),
> +                ('%s' % UiStrings().BibleShortSearch))
> +            return None
> +        elif text:
>              return self.db_cache[bible].verse_search(text)
>          else:
> -            self.main_window.information_message(
> -                translate('BiblesPlugin.BibleManager', 'Scripture
> Reference Error'),
> -                translate('BiblesPlugin.BibleManager', 'You did not enter
> a search keyword.\nYou can separate '
> -                          'different keywords by a space to search for
> all of your keywords and you can separate '
> -                          'them by a comma to search for one of them.')
> -            )
>              return None
>
> +    def get_verses_combined(self, bible, verse_text, book_ref_id=False,
> show_error=False):
> +            log.debug('BibleManager.get_verses("%s", "%s")', bible,
> verse_text)
> +            if not bible:
> +                if show_error:
> +                    self.main_window.information_message(
> +                        translate('BiblesPlugin.BibleManager', 'No Bibles
> Available'),
> +                        translate('BiblesPlugin.BibleManager', 'There are
> no Bibles currently installed. Please use the'
> +                                  ' Import Wizard to install one or more
> Bibles.')
> +                    )
> +                return None
> +            language_selection = self.get_language_selection(bible)
> +            ref_list = parse_reference(verse_text, self.db_cache[bible],
> language_selection, book_ref_id)
> +            if ref_list:
> +                return self.db_cache[bible].get_verses(ref_list,
> show_error)
> +            else:
> +                return None
> +
>      def save_meta_data(self, bible, version, copyright, permissions,
> book_name_language=None):
>          """
>          Saves the bibles meta data.
>
> === modified file 'openlp/plugins/bibles/lib/mediaitem.py'
> --- openlp/plugins/bibles/lib/mediaitem.py      2016-01-08 17:44:47 +0000
> +++ openlp/plugins/bibles/lib/mediaitem.py      2016-04-04 01:13:54 +0000
> @@ -45,6 +45,7 @@
>      """
>      Reference = 1
>      Text = 2
> +    Combined = 3
>
>
>  class BibleMediaItem(MediaManagerItem):
> @@ -309,6 +310,9 @@
>          self.plugin.manager.media = self
>          self.load_bibles()
>          self.quick_search_edit.set_search_types([
> +            (BibleSearch.Combined, ':/bibles/bibles_search_combined.png',
> +                translate('BiblesPlugin.MediaItem', 'Text or Scripture
> Reference'),
> +                translate('BiblesPlugin.MediaItem', 'Text or Scripture
> Reference...')),
>              (BibleSearch.Reference,
> ':/bibles/bibles_search_reference.png',
>                  translate('BiblesPlugin.MediaItem', 'Scripture
> Reference'),
>                  translate('BiblesPlugin.MediaItem', 'Search Scripture
> Reference...')),
> @@ -423,7 +427,7 @@
>      def update_auto_completer(self):
>          """
>          This updates the bible book completion list for the search field.
> The completion depends on the bible. It is
> -        only updated when we are doing a reference search, otherwise the
> auto completion list is removed.
> +        only updated when we are doing reference or combined search, in
> text search the completion list is removed.
>          """
>          log.debug('update_auto_completer')
>          # Save the current search type to the configuration.
> @@ -431,8 +435,8 @@
>          # Save the current bible to the configuration.
>          Settings().setValue(self.settings_section + '/quick bible',
> self.quickVersionComboBox.currentText())
>          books = []
> -        # We have to do a 'Reference Search'.
> -        if self.quick_search_edit.current_search_type() ==
> BibleSearch.Reference:
> +        # We have to do a 'Reference Search' (Or as part of Combined
> Search).
> +        if self.quick_search_edit.current_search_type() is not
> BibleSearch.Text:
>              bibles = self.plugin.manager.get_bibles()
>              bible = self.quickVersionComboBox.currentText()
>              if bible:
> @@ -648,10 +652,59 @@
>          self.check_search_result()
>          self.application.set_normal_cursor()
>
> +    def on_quick_reference_search(self):
> +        # We are doing a 'Reference Search'.
> +        bible = self.quickVersionComboBox.currentText()
> +        second_bible = self.quickSecondComboBox.currentText()
> +        # Get input from field and replace '. ' with ''
> +        # This will check if field has any '.' and removes them. Eg. Gen.
> 1 = Gen 1 = Genesis 1
> +        text_direct = self.quick_search_edit.text()
> +        text = text_direct.replace('. ', ' ')
> +        if self.quick_search_edit.current_search_type() ==
> BibleSearch.Reference:
> +            self.search_results = self.plugin.manager.get_verses(bible,
> text)
> +        else:
> +            self.search_results =
> self.plugin.manager.get_verses_combined(bible, text)
> +        if second_bible and self.search_results:
> +            self.second_search_results = \
> +                self.plugin.manager.get_verses(second_bible, text,
> self.search_results[0].book.book_reference_id)
> +
> +    def on_quick_text_search(self):
> +        # We are doing a 'Text Search'.
> +        bible = self.quickVersionComboBox.currentText()
> +        second_bible = self.quickSecondComboBox.currentText()
> +        text = self.quick_search_edit.text()
> +        self.application.set_busy_cursor()
> +        bibles = self.plugin.manager.get_bibles()
> +        self.search_results = self.plugin.manager.verse_search(bible,
> second_bible, text)
> +        if second_bible and self.search_results:
> +            text = []
> +            new_search_results = []
> +            count = 0
> +            passage_not_found = False
> +            for verse in self.search_results:
> +                db_book =
> bibles[second_bible].get_book_by_book_ref_id(verse.book.book_reference_id)
> +                if not db_book:
> +                    log.debug('Passage "%s %d:%d" not found in Second
> Bible' %
> +                              (verse.book.name, verse.chapter,
> verse.verse))
> +                    passage_not_found = True
> +                    count += 1
> +                    continue
> +                new_search_results.append(verse)
> +                text.append((verse.book.book_reference_id, verse.chapter,
> verse.verse, verse.verse))
> +            if passage_not_found:
> +                self.main_window.information_message(
> +                    translate('BiblesPlugin.MediaItem', 'Information'),
> +                    translate('BiblesPlugin.MediaItem', 'The second Bible
> does not contain all the verses '
> +                                                        'that are in the
> main Bible. Only verses found in both Bibles '
> +                                                        'will be shown.
> %d verses have not been included '
> +                                                        'in the
> results.') % count)
> +            self.search_results = new_search_results
> +            self.second_search_results =
> bibles[second_bible].get_verses(text)
> +
>      def on_quick_search_button(self):
>          """
> -        Does a quick search and saves the search results. Quick search
> can either be "Reference Search" or
> -        "Text Search".
> +        Does a quick search and saves the search results. Quick search
> can be:
> +        "Reference Search" or "Text Search" or Combined search.
>          """
>          log.debug('Quick Search Button clicked')
>          self.quickSearchButton.setEnabled(False)
> @@ -660,40 +713,49 @@
>          second_bible = self.quickSecondComboBox.currentText()
>          text = self.quick_search_edit.text()
>          if self.quick_search_edit.current_search_type() ==
> BibleSearch.Reference:
> -            # We are doing a 'Reference Search'.
> -            self.search_results = self.plugin.manager.get_verses(bible,
> text)
> -            if second_bible and self.search_results:
> -                self.second_search_results = \
> -                    self.plugin.manager.get_verses(second_bible, text,
> self.search_results[0].book.book_reference_id)
> -        else:
> -            # We are doing a 'Text Search'.
> -            self.application.set_busy_cursor()
> -            bibles = self.plugin.manager.get_bibles()
> -            self.search_results = self.plugin.manager.verse_search(bible,
> second_bible, text)
> -            if second_bible and self.search_results:
> -                text = []
> -                new_search_results = []
> -                count = 0
> -                passage_not_found = False
> -                for verse in self.search_results:
> -                    db_book =
> bibles[second_bible].get_book_by_book_ref_id(verse.book.book_reference_id)
> -                    if not db_book:
> -                        log.debug('Passage "%s %d:%d" not found in Second
> Bible' %
> -                                  (verse.book.name, verse.chapter,
> verse.verse))
> -                        passage_not_found = True
> -                        count += 1
> -                        continue
> -                    new_search_results.append(verse)
> -                    text.append((verse.book.book_reference_id,
> verse.chapter, verse.verse, verse.verse))
> -                if passage_not_found:
> -                    QtWidgets.QMessageBox.information(
> -                        self, translate('BiblesPlugin.MediaItem',
> 'Information'),
> -                        translate('BiblesPlugin.MediaItem', 'The second
> Bible does not contain all the verses '
> -                                  'that are in the main Bible. Only
> verses found in both Bibles will be shown. %d '
> -                                  'verses have not been included in the
> results.') % count,
> -
> QtWidgets.QMessageBox.StandardButtons(QtWidgets.QMessageBox.Ok))
> -                self.search_results = new_search_results
> -                self.second_search_results =
> bibles[second_bible].get_verses(text)
> +            # We are doing a 'Reference Search'. (Get script from def
> on_quick_reference_search)
> +            self.on_quick_reference_search()
> +        elif self.quick_search_edit.current_search_type() ==
> BibleSearch.Text:
> +            # We are doing a 'Text Search'. (Get script from def
> on_quick_text_search)
> +            self.on_quick_text_search()
> +        # Combined search, starting with reference search.
> +        elif self.quick_search_edit.current_search_type() ==
> BibleSearch.Combined:
> +            self.on_quick_reference_search()
> +        # If keyword is shorter than 3 (not including spaces), message is
> given and search is finalized.
> +        # It's actually to find verses with less than 3 chars (Eg. G1 =
> Genesis 1) thus this error is not shown if
> +        # any results are found. This check needs to be here in order to
> avoid duplicate errors.
> +            if not self.search_results and len(text) - text.count(' ') <
> 3:
> +                self.main_window.information_message(
> +                    ('%s' % UiStrings().BibleShortSearchTitle),
> +                    ('%s' % UiStrings().BibleShortSearch))
> +            # Text search starts here if no reference was found and
> keyword is longer than 2.
> +            # This is required in order to avoid duplicate error messages
> for short keywords.
> +            if not self.search_results and len(text) - text.count(' ') >
> 2:
> +                self.on_quick_text_search()
> +                # If no Text or Reference is found, message is given.
> +                if not self.search_results:
> +                        self.application.set_normal_cursor()
> +                        reference_separators = {
> +                            'verse':
> get_reference_separator('sep_v_display'),
> +                            'range':
> get_reference_separator('sep_r_display'),
> +                            'list':
> get_reference_separator('sep_l_display')}
> +
> self.main_window.information_message(translate('BiblesPlugin.BibleManager',
> 'Nothing found'),
> +
>  translate('BiblesPlugin.BibleManager', '<strong>OpenLP '
> +
>                           'couldn’t find '
> +
>                           'anything with your'
> +
>                           ' search.</strong>'
> +
>                           '<br><br>'
> +
>                           'If you tried to '
> +
>                           'search with '
> +
>                           'Scripture '
> +
>                           'Reference, please '
> +
>                           'make<br>sure that '
> +
>                           'your reference '
> +
>                           'follows one of '
> +
>                           'these patterns:%s'
> +                                                                       %
> UiStrings().BibleScriptureError %
> +
>  reference_separators))
> +        # Finalizing the search
>          if not self.quickLockButton.isChecked():
>              self.list_view.clear()
>          if self.list_view.count() != 0 and self.search_results:
>
> === modified file 'resources/images/openlp-2.qrc'
> --- resources/images/openlp-2.qrc       2016-02-06 17:50:58 +0000
> +++ resources/images/openlp-2.qrc       2016-04-04 01:13:54 +0000
> @@ -30,6 +30,7 @@
>      <file>image_new_group.png</file>
>    </qresource>
>    <qresource prefix="bibles">
> +    <file>bibles_search_combined.png</file>
>      <file>bibles_search_text.png</file>
>      <file>bibles_search_reference.png</file>
>      <file>bibles_upgrade_alert.png</file>
>
>
>