Merge lp:~trb143/openlp/splitter into lp:openlp

Proposed by Tim Bentley
Status: Superseded
Proposed branch: lp:~trb143/openlp/splitter
Merge into: lp:openlp
Diff against target: 412 lines (+122/-60)
10 files modified
openlp/core/common/uistrings.py (+1/-1)
openlp/core/lib/renderer.py (+3/-0)
openlp/core/ui/lib/listpreviewwidget.py (+5/-5)
openlp/plugins/songs/forms/duplicatesongremovalform.py (+1/-2)
openlp/plugins/songs/forms/editversedialog.py (+13/-6)
openlp/plugins/songs/forms/editverseform.py (+24/-9)
openlp/plugins/songs/lib/__init__.py (+34/-33)
openlp/plugins/songs/lib/mediaitem.py (+7/-3)
openlp/plugins/songs/lib/openlyricsxml.py (+6/-1)
tests/functional/openlp_plugins/songs/test_editverseform.py (+28/-0)
To merge this branch: bzr merge lp:~trb143/openlp/splitter
Reviewer Review Type Date Requested Status
Tomas Groth Pending
Review via email: mp+331616@code.launchpad.net

This proposal supersedes a proposal from 2017-09-29.

This proposal has been superseded by a proposal from 2017-10-01.

Description of the change

Add option to add a split to a song
This will split the verse when added to the service but keep the verse together for editing.
Useful for the 10 line hymn verses which need 2 slides to display.

Fix some iffy spelling

lp:~trb143/openlp/splitter (revision 2738)
[SUCCESS] https://ci.openlp.io/job/Branch-01-Pull/2182/
[SUCCESS] https://ci.openlp.io/job/Branch-02-Functional-Tests/2085/
[SUCCESS] https://ci.openlp.io/job/Branch-03-Interface-Tests/1972/
[SUCCESS] https://ci.openlp.io/job/Branch-04a-Code_Analysis/1342/
[SUCCESS] https://ci.openlp.io/job/Branch-04b-Test_Coverage/1178/
[SUCCESS] https://ci.openlp.io/job/Branch-04c-Code_Analysis2/308/
[FAILURE] https://ci.openlp.io/job/Branch-05-AppVeyor-Tests/150/

To post a comment you must log in.
Revision history for this message
Raoul Snyman (raoul-snyman) wrote : Posted in a previous version of this proposal

As per IRC, mrs_fly's suggestion was "Auto Split" and "Force Split". I'm going to e-mail the mailing list and see what others think.

Revision history for this message
Tomas Groth (tomasgroth) wrote : Posted in a previous version of this proposal

The openlyrics export does handle the new spilt, which I believe it should. The import should of course also be able to restore it. I'll admit I'm not sure how it should be done...

review: Needs Fixing
Revision history for this message
Raoul Snyman (raoul-snyman) wrote : Posted in a previous version of this proposal

Let's call the buttons "Overflow Split" and "Forced Split"

Revision history for this message
Tomas Groth (tomasgroth) wrote : Posted in a previous version of this proposal

The openlyrics import/export still needs to handle the new spilt...

review: Needs Fixing
lp:~trb143/openlp/splitter updated
2742. By Tim Bentley

head

Unmerged revisions

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'openlp/core/common/uistrings.py'
2--- openlp/core/common/uistrings.py 2017-09-06 21:36:31 +0000
3+++ openlp/core/common/uistrings.py 2017-10-01 20:11:27 +0000
4@@ -147,7 +147,7 @@
5 self.SaveService = translate('OpenLP.Ui', 'Save Service')
6 self.Service = translate('OpenLP.Ui', 'Service')
7 self.ShortResults = translate('OpenLP.Ui', 'Please type more text to use \'Search As You Type\'')
8- self.Split = translate('OpenLP.Ui', 'Optional &Split')
9+ self.Split = translate('OpenLP.Ui', 'Overflow &Split')
10 self.SplitToolTip = translate('OpenLP.Ui',
11 'Split a slide into two only if it does not fit on the screen as one slide.')
12 self.StartingImport = translate('OpenLP.Ui', 'Starting import...')
13
14=== modified file 'openlp/core/lib/renderer.py'
15--- openlp/core/lib/renderer.py 2017-09-26 16:39:13 +0000
16+++ openlp/core/lib/renderer.py 2017-10-01 20:11:27 +0000
17@@ -243,6 +243,9 @@
18 elif item.is_capable(ItemCapabilities.CanSoftBreak):
19 pages = []
20 if '[---]' in text:
21+ # Remove Overflow split if at start of the text
22+ if text.startswith('[---]'):
23+ text = text[5:]
24 # Remove two or more option slide breaks next to each other (causing infinite loop).
25 while '\n[---]\n[---]\n' in text:
26 text = text.replace('\n[---]\n[---]\n', '\n[---]\n')
27
28=== modified file 'openlp/core/ui/lib/listpreviewwidget.py'
29--- openlp/core/ui/lib/listpreviewwidget.py 2016-12-31 11:01:36 +0000
30+++ openlp/core/ui/lib/listpreviewwidget.py 2017-10-01 20:11:27 +0000
31@@ -209,21 +209,21 @@
32 Switches to the given row.
33 """
34 # Retrieve setting
35- autoscrolling = Settings().value('advanced/autoscrolling')
36+ auto_scrolling = Settings().value('advanced/autoscrolling')
37 # Check if auto-scroll disabled (None) and validate value as dict containing 'dist' and 'pos'
38 # 'dist' represents the slide to scroll to relative to the new slide (-1 = previous, 0 = current, 1 = next)
39 # 'pos' represents the vert position of of the slide (0 = in view, 1 = top, 2 = middle, 3 = bottom)
40- if not (isinstance(autoscrolling, dict) and 'dist' in autoscrolling and 'pos' in autoscrolling and
41- isinstance(autoscrolling['dist'], int) and isinstance(autoscrolling['pos'], int)):
42+ if not (isinstance(auto_scrolling, dict) and 'dist' in auto_scrolling and 'pos' in auto_scrolling and
43+ isinstance(auto_scrolling['dist'], int) and isinstance(auto_scrolling['pos'], int)):
44 return
45 # prevent scrolling past list bounds
46- scroll_to_slide = slide + autoscrolling['dist']
47+ scroll_to_slide = slide + auto_scrolling['dist']
48 if scroll_to_slide < 0:
49 scroll_to_slide = 0
50 if scroll_to_slide >= self.slide_count():
51 scroll_to_slide = self.slide_count() - 1
52 # Scroll to item if possible.
53- self.scrollToItem(self.item(scroll_to_slide, 0), autoscrolling['pos'])
54+ self.scrollToItem(self.item(scroll_to_slide, 0), auto_scrolling['pos'])
55 self.selectRow(slide)
56
57 def current_slide_number(self):
58
59=== modified file 'openlp/plugins/songs/forms/duplicatesongremovalform.py'
60--- openlp/plugins/songs/forms/duplicatesongremovalform.py 2017-06-09 06:06:49 +0000
61+++ openlp/plugins/songs/forms/duplicatesongremovalform.py 2017-10-01 20:11:27 +0000
62@@ -25,14 +25,13 @@
63
64 import logging
65 import multiprocessing
66-import os
67
68 from PyQt5 import QtCore, QtWidgets
69
70 from openlp.core.common import Registry, RegistryProperties, translate
71 from openlp.core.ui.lib.wizard import OpenLPWizard, WizardStrings
72 from openlp.plugins.songs.lib import delete_song
73-from openlp.plugins.songs.lib.db import Song, MediaFile
74+from openlp.plugins.songs.lib.db import Song
75 from openlp.plugins.songs.forms.songreviewwidget import SongReviewWidget
76 from openlp.plugins.songs.lib.songcompare import songs_probably_equal
77
78
79=== modified file 'openlp/plugins/songs/forms/editversedialog.py'
80--- openlp/plugins/songs/forms/editversedialog.py 2017-07-04 23:13:51 +0000
81+++ openlp/plugins/songs/forms/editversedialog.py 2017-10-01 20:11:27 +0000
82@@ -42,10 +42,14 @@
83 self.dialog_layout.addWidget(self.verse_text_edit)
84 self.verse_type_layout = QtWidgets.QHBoxLayout()
85 self.verse_type_layout.setObjectName('verse_type_layout')
86- self.split_button = QtWidgets.QPushButton(edit_verse_dialog)
87- self.split_button.setIcon(build_icon(':/general/general_add.png'))
88- self.split_button.setObjectName('split_button')
89- self.verse_type_layout.addWidget(self.split_button)
90+ self.forced_split_button = QtWidgets.QPushButton(edit_verse_dialog)
91+ self.forced_split_button.setIcon(build_icon(':/general/general_add.png'))
92+ self.forced_split_button.setObjectName('forced_split_button')
93+ self.verse_type_layout.addWidget(self.forced_split_button)
94+ self.overflow_split_button = QtWidgets.QPushButton(edit_verse_dialog)
95+ self.overflow_split_button.setIcon(build_icon(':/general/general_add.png'))
96+ self.overflow_split_button.setObjectName('overflow_split_button')
97+ self.verse_type_layout.addWidget(self.overflow_split_button)
98 self.verse_type_label = QtWidgets.QLabel(edit_verse_dialog)
99 self.verse_type_label.setObjectName('verse_type_label')
100 self.verse_type_layout.addWidget(self.verse_type_label)
101@@ -93,8 +97,11 @@
102 self.verse_type_combo_box.setItemText(VerseType.Intro, VerseType.translated_names[VerseType.Intro])
103 self.verse_type_combo_box.setItemText(VerseType.Ending, VerseType.translated_names[VerseType.Ending])
104 self.verse_type_combo_box.setItemText(VerseType.Other, VerseType.translated_names[VerseType.Other])
105- self.split_button.setText(UiStrings().Split)
106- self.split_button.setToolTip(UiStrings().SplitToolTip)
107+ self.overflow_split_button.setText(UiStrings().Split)
108+ self.overflow_split_button.setToolTip(UiStrings().SplitToolTip)
109+ self.forced_split_button.setText(translate('SongsPlugin.EditVerseForm', '&Forced Split'))
110+ self.forced_split_button.setToolTip(translate('SongsPlugin.EditVerseForm', 'Split the verse when displayed '
111+ 'regardless of the screen size.'))
112 self.insert_button.setText(translate('SongsPlugin.EditVerseForm', '&Insert'))
113 self.insert_button.setToolTip(translate('SongsPlugin.EditVerseForm',
114 'Split a slide into two by inserting a verse splitter.'))
115
116=== modified file 'openlp/plugins/songs/forms/editverseform.py'
117--- openlp/plugins/songs/forms/editverseform.py 2017-06-04 12:14:23 +0000
118+++ openlp/plugins/songs/forms/editverseform.py 2017-10-01 20:11:27 +0000
119@@ -48,12 +48,13 @@
120 self.setupUi(self)
121 self.has_single_verse = False
122 self.insert_button.clicked.connect(self.on_insert_button_clicked)
123- self.split_button.clicked.connect(self.on_split_button_clicked)
124+ self.overflow_split_button.clicked.connect(self.on_overflow_split_button_clicked)
125 self.verse_text_edit.cursorPositionChanged.connect(self.on_cursor_position_changed)
126 self.verse_type_combo_box.currentIndexChanged.connect(self.on_verse_type_combo_box_changed)
127+ self.forced_split_button.clicked.connect(self.on_forced_split_button_clicked)
128 if Settings().value('songs/enable chords'):
129- self.transpose_down_button.clicked.connect(self.on_transepose_down_button_clicked)
130- self.transpose_up_button.clicked.connect(self.on_transepose_up_button_clicked)
131+ self.transpose_down_button.clicked.connect(self.on_transpose_down_button_clicked)
132+ self.transpose_up_button.clicked.connect(self.on_transpose_up_button_clicked)
133
134 def insert_verse(self, verse_tag, verse_num=1):
135 """
136@@ -68,13 +69,27 @@
137 self.verse_text_edit.insertPlainText('---[{tag}:{number}]---\n'.format(tag=verse_tag, number=verse_num))
138 self.verse_text_edit.setFocus()
139
140- def on_split_button_clicked(self):
141- """
142- The split button has been pressed
143+ def on_overflow_split_button_clicked(self):
144+ """
145+ The optional split button has been pressed so we need add the split
146+ """
147+ self._add_splitter_to_text('[---]')
148+
149+ def on_forced_split_button_clicked(self):
150+ """
151+ The force split button has been pressed so we need add the split
152+ """
153+ self._add_splitter_to_text('[--}{--]')
154+
155+ def _add_splitter_to_text(self, insert_string):
156+ """
157+ Add a custom splitter to the song text
158+
159+ :param insert_string: The string to insert
160+ :return:
161 """
162 text = self.verse_text_edit.toPlainText()
163 position = self.verse_text_edit.textCursor().position()
164- insert_string = '[---]'
165 if position and text[position - 1] != '\n':
166 insert_string = '\n' + insert_string
167 if position == len(text) or text[position] != '\n':
168@@ -101,7 +116,7 @@
169 """
170 self.update_suggested_verse_number()
171
172- def on_transepose_up_button_clicked(self):
173+ def on_transpose_up_button_clicked(self):
174 """
175 The transpose up button clicked
176 """
177@@ -118,7 +133,7 @@
178 self.verse_text_edit.setFocus()
179 self.verse_text_edit.moveCursor(QtGui.QTextCursor.End)
180
181- def on_transepose_down_button_clicked(self):
182+ def on_transpose_down_button_clicked(self):
183 """
184 The transpose down button clicked
185 """
186
187=== modified file 'openlp/plugins/songs/lib/__init__.py'
188--- openlp/plugins/songs/lib/__init__.py 2017-09-30 20:16:30 +0000
189+++ openlp/plugins/songs/lib/__init__.py 2017-10-01 20:11:27 +0000
190@@ -546,12 +546,12 @@
191 song_plugin.manager.delete_object(Song, song_id)
192
193
194-def transpose_lyrics(lyrics, transepose_value):
195+def transpose_lyrics(lyrics, transpose_value):
196 """
197- Transepose lyrics
198+ Transpose lyrics
199
200- :param lyrcs: The lyrics to be transposed
201- :param transepose_value: The value to transpose the lyrics with
202+ :param lyrics: The lyrics to be transposed
203+ :param transpose_value: The value to transpose the lyrics with
204 :return: The transposed lyrics
205 """
206 # Split text by verse delimiter - both normal and optional
207@@ -562,16 +562,17 @@
208 if verse.startswith('---[') or verse == '[---]':
209 transposed_lyrics += verse
210 else:
211- transposed_lyrics += transpose_verse(verse, transepose_value, notation)
212+ transposed_lyrics += transpose_verse(verse, transpose_value, notation)
213 return transposed_lyrics
214
215
216-def transpose_verse(verse_text, transepose_value, notation):
217+def transpose_verse(verse_text, transpose_value, notation):
218 """
219- Transepose lyrics
220+ Transpose Verse
221
222- :param lyrcs: The lyrics to be transposed
223- :param transepose_value: The value to transpose the lyrics with
224+ :param verse_text: The lyrics to be transposed
225+ :param transpose_value: The value to transpose the lyrics with
226+ :param notation: which notation to use
227 :return: The transposed lyrics
228 """
229 if '[' not in verse_text:
230@@ -589,11 +590,11 @@
231 if word == ']':
232 in_tag = False
233 transposed_lyrics += word
234- elif word == '/':
235+ elif word == '/' or word == '--}{--':
236 transposed_lyrics += word
237 else:
238 # This MUST be a chord
239- transposed_lyrics += transpose_chord(word, transepose_value, notation)
240+ transposed_lyrics += transpose_chord(word, transpose_value, notation)
241 # If still inside a chord tag something is wrong!
242 if in_tag:
243 return verse_text
244@@ -629,36 +630,36 @@
245 for i in range(0, len(chord_split)):
246 if i > 0:
247 transposed_chord += '/'
248- currentchord = chord_split[i]
249- if currentchord and currentchord[0] == '(':
250+ current_chord = chord_split[i]
251+ if current_chord and current_chord[0] == '(':
252 transposed_chord += '('
253- if len(currentchord) > 1:
254- currentchord = currentchord[1:]
255+ if len(current_chord) > 1:
256+ current_chord = current_chord[1:]
257 else:
258- currentchord = ''
259- if len(currentchord) > 0:
260- if len(currentchord) > 1:
261- if '#b'.find(currentchord[1]) == -1:
262- note = currentchord[0:1]
263- rest = currentchord[1:]
264+ current_chord = ''
265+ if len(current_chord) > 0:
266+ if len(current_chord) > 1:
267+ if '#b'.find(current_chord[1]) == -1:
268+ note = current_chord[0:1]
269+ rest = current_chord[1:]
270 else:
271- note = currentchord[0:2]
272- rest = currentchord[2:]
273+ note = current_chord[0:2]
274+ rest = current_chord[2:]
275 else:
276- note = currentchord
277+ note = current_chord
278 rest = ''
279- notenumber = notes_flat.index(note) if note not in notes_sharp else notes_sharp.index(note)
280- notenumber += transpose_value
281- while notenumber > 11:
282- notenumber -= 12
283- while notenumber < 0:
284- notenumber += 12
285+ note_number = notes_flat.index(note) if note not in notes_sharp else notes_sharp.index(note)
286+ note_number += transpose_value
287+ while note_number > 11:
288+ note_number -= 12
289+ while note_number < 0:
290+ note_number += 12
291 if i == 0:
292- current_chord = notes_sharp[notenumber] if notes_preferred[notenumber] == '#' else notes_flat[
293- notenumber]
294+ current_chord = notes_sharp[note_number] if notes_preferred[note_number] == '#' else notes_flat[
295+ note_number]
296 last_chord = current_chord
297 else:
298- current_chord = notes_flat[notenumber] if last_chord not in notes_sharp else notes_sharp[notenumber]
299+ current_chord = notes_flat[note_number] if last_chord not in notes_sharp else notes_sharp[note_number]
300 if not (note not in notes_flat and note not in notes_sharp):
301 transposed_chord += current_chord + rest
302 else:
303
304=== modified file 'openlp/plugins/songs/lib/mediaitem.py'
305--- openlp/plugins/songs/lib/mediaitem.py 2017-09-30 20:16:30 +0000
306+++ openlp/plugins/songs/lib/mediaitem.py 2017-10-01 20:11:27 +0000
307@@ -577,7 +577,7 @@
308 if not song.verse_order.strip():
309 for verse in verse_list:
310 # We cannot use from_loose_input() here, because database is supposed to contain English lowercase
311- # singlechar tags.
312+ # single char tags.
313 verse_tag = verse[0]['type']
314 verse_index = None
315 if len(verse_tag) > 1:
316@@ -588,7 +588,9 @@
317 verse_index = VerseType.from_tag(verse_tag)
318 verse_tag = VerseType.translated_tags[verse_index].upper()
319 verse_def = '{tag}{label}'.format(tag=verse_tag, label=verse[0]['label'])
320- service_item.add_from_text(str(verse[1]), verse_def)
321+ force_verse = verse[1].split('[--}{--]\n')
322+ for split_verse in force_verse:
323+ service_item.add_from_text(split_verse, verse_def)
324 else:
325 # Loop through the verse list and expand the song accordingly.
326 for order in song.verse_order.lower().split():
327@@ -603,7 +605,9 @@
328 verse_index = VerseType.from_tag(verse[0]['type'])
329 verse_tag = VerseType.translated_tags[verse_index]
330 verse_def = '{tag}{label}'.format(tag=verse_tag, label=verse[0]['label'])
331- service_item.add_from_text(verse[1], verse_def)
332+ force_verse = verse[1].split('[--}{--]\n')
333+ for split_verse in force_verse:
334+ service_item.add_from_text(split_verse, verse_def)
335 service_item.title = song.title
336 author_list = self.generate_footer(service_item, song)
337 service_item.data_string = {'title': song.search_title, 'authors': ', '.join(author_list)}
338
339=== modified file 'openlp/plugins/songs/lib/openlyricsxml.py'
340--- openlp/plugins/songs/lib/openlyricsxml.py 2017-09-13 06:08:38 +0000
341+++ openlp/plugins/songs/lib/openlyricsxml.py 2017-10-01 20:11:27 +0000
342@@ -71,6 +71,7 @@
343
344 NAMESPACE = 'http://openlyrics.info/namespace/2009/song'
345 NSMAP = '{{' + NAMESPACE + '}}{tag}'
346+NEWPAGETAG = '<p style="page-break-after: always;"/>'
347
348
349 class SongXML(object):
350@@ -281,7 +282,7 @@
351 tags_element = None
352 match = re.search('\{/?\w+\}', song.lyrics, re.UNICODE)
353 if match:
354- # Named 'format_' - 'format' is built-in fuction in Python.
355+ # Named 'format_' - 'format' is built-in function in Python.
356 format_ = etree.SubElement(song_xml, 'format')
357 tags_element = etree.SubElement(format_, 'tags')
358 tags_element.set('application', 'OpenLP')
359@@ -472,6 +473,7 @@
360 text = text.replace('{{/{tag}}}'.format(tag=tag), '</tag>')
361 # Replace \n with <br/>.
362 text = text.replace('\n', '<br/>')
363+ text = text.replace('[--}{--]', NEWPAGETAG)
364 element = etree.XML('<lines>{text}</lines>'.format(text=text))
365 verse_element.append(element)
366 return element
367@@ -634,6 +636,9 @@
368 if element.tail:
369 text += element.tail
370 return text
371+ elif newlines and element.tag == NSMAP.format(tag='p') and 'page-break-after' in str(element.attrib):
372+ text += '[--}{--]'
373+ return text
374 # Start formatting tag.
375 if element.tag == NSMAP.format(tag='tag'):
376 text += '{{{name}}}'.format(name=element.get('name'))
377
378=== modified file 'tests/functional/openlp_plugins/songs/test_editverseform.py'
379--- tests/functional/openlp_plugins/songs/test_editverseform.py 2017-05-30 20:06:27 +0000
380+++ tests/functional/openlp_plugins/songs/test_editverseform.py 2017-10-01 20:11:27 +0000
381@@ -72,3 +72,31 @@
382
383 # THEN the verse number must not be changed
384 self.assertEqual(3, self.edit_verse_form.verse_number_box.value(), 'The verse number should be 3')
385+
386+ def test_on_divide_split_button_clicked(self):
387+ """
388+ Test that divide adds text at the correct position
389+ """
390+ # GIVEN some input values
391+ self.edit_verse_form.verse_type_combo_box.currentIndex = MagicMock(return_value=4)
392+ self.edit_verse_form.verse_text_edit.setPlainText('Text\n')
393+
394+ # WHEN the method is called
395+ self.edit_verse_form.on_forced_split_button_clicked()
396+ # THEN the verse number must not be changed
397+ self.assertEqual('[--}{--]\nText\n', self.edit_verse_form.verse_text_edit.toPlainText(),
398+ 'The verse number should be [--}{--]\nText\n')
399+
400+ def test_on_split_button_clicked(self):
401+ """
402+ Test that divide adds text at the correct position
403+ """
404+ # GIVEN some input values
405+ self.edit_verse_form.verse_type_combo_box.currentIndex = MagicMock(return_value=4)
406+ self.edit_verse_form.verse_text_edit.setPlainText('Text\n')
407+
408+ # WHEN the method is called
409+ self.edit_verse_form.on_overflow_split_button_clicked()
410+ # THEN the verse number must not be changed
411+ self.assertEqual('[---]\nText\n', self.edit_verse_form.verse_text_edit.toPlainText(),
412+ 'The verse number should be [---]\nText\n')