openlp-core team mailing list archive
-
openlp-core team
-
Mailing list archive
-
Message #29091
[Merge] lp:~suutari-olli/openlp/combined-bible-quick-search into lp:openlp
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.
--
Your team OpenLP Core is subscribed to branch 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>
Follow ups
