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 protected]> 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> > > > -- https://code.launchpad.net/~suutari-olli/openlp/combined-bible-quick-search/+merge/290846 Your team OpenLP Core is subscribed to branch lp:openlp. _______________________________________________ Mailing list: https://launchpad.net/~openlp-core Post to : [email protected] Unsubscribe : https://launchpad.net/~openlp-core More help : https://help.launchpad.net/ListHelp
