openlp-core team mailing list archive

[Azaziah <suutari.olli@xxxxxxxxx>]

Could you explain "Cannot test as the UI is not visible." ?

I replied to the other comments, thank you for the review.

Diff comments:

> === modified file 'openlp/core/lib/renderer.py'
> --- openlp/core/lib/renderer.py	2016-01-23 12:38:08 +0000
> +++ openlp/core/lib/renderer.py	2016-05-09 19:57:28 +0000
> @@ -249,8 +249,8 @@
>                  while '[---] ' in text:
>                      text = text.replace('[---] ', '[---]')
>                  count = 0
> -                # only loop 5 times as there will never be more than 5 incorrect logical splits on a single slide.
> -                while True and count < 5:
> +                # only loop 50 times as there will never be more than 50 optional splits on a single slide.
> +                while True and count < 51:

That makes sense, but some people prefer to use large font sides and thus higher cap is required. Currently if this limit is reached, the song won't be visible properly. I can't see the downside of increasing this since it seems to fix problems rather than causing them. Is there any reason why this should not be increased?

>                      slides = text.split('\n[---]\n', 2)
>                      # If there are (at least) two occurrences of [---] we use the first two slides (and neglect the last
>                      # for now).
> 
> === modified file 'openlp/plugins/songs/forms/editsongform.py'
> --- openlp/plugins/songs/forms/editsongform.py	2016-04-25 11:01:32 +0000
> +++ openlp/plugins/songs/forms/editsongform.py	2016-05-09 19:57:28 +0000
> @@ -776,6 +776,55 @@
>                          item = QtWidgets.QTableWidgetItem(entry, 0)
>                          item.setData(QtCore.Qt.UserRole, temp_ids[row])
>                          self.verse_list_widget.setItem(row, 0, item)
> +        # This code converts force splits aka. Verse tags inserted during edit to new verses from text.
> +        # It is largely copied from def on_verse_edit_all_button_clicked

If you add verses, eg. ---[Verse:1]--- this code will convert it to a new slide. If this is not done, the tags won't be converted and will be shown in Live and everything will be on the same verse. Some un-required parts of that code were removed.
This code is triggered when the verse is saved in "Edit single verse mode" The same code re-factors the "Edit all verses" text
Can't see why this should be modified because it does the same thing.

> +        verse_list = ''
> +        if self.verse_list_widget.rowCount() > 0:
> +            for row in range(self.verse_list_widget.rowCount()):
> +                item = self.verse_list_widget.item(row, 0)
> +                field = item.data(QtCore.Qt.UserRole)
> +                verse_tag = VerseType.translated_name(field[0])
> +                verse_num = field[1:]
> +                verse_list += '---[%s:%s]---\n' % (verse_tag, verse_num)
> +                verse_list += item.text()
> +                verse_list += '\n'
> +            self.verse_form.set_verse(verse_list)
> +        verse_list = self.verse_form.get_all_verses()
> +        verse_list = str(verse_list.replace('\r\n', '\n'))
> +        self.verse_list_widget.clear()
> +        self.verse_list_widget.setRowCount(0)
> +        for row in self.find_verse_split.split(verse_list):
> +            for match in row.split('---['):
> +                for count, parts in enumerate(match.split(']---\n')):
> +                    if count == 0:
> +                        if len(parts) == 0:
> +                            continue
> +                        # handling carefully user inputted versetags
> +                        separator = parts.find(':')
> +                        if separator >= 0:
> +                            verse_name = parts[0:separator].strip()
> +                            verse_num = parts[separator + 1:].strip()
> +                        else:
> +                            verse_name = parts
> +                            verse_num = '1'
> +                        verse_index = VerseType.from_loose_input(verse_name)
> +                        verse_tag = VerseType.tags[verse_index]
> +                        # Later we need to handle v1a as well.
> +                        regex = re.compile(r'\D*(\d+)\D*')
> +                        match = regex.match(verse_num)
> +                        if match:
> +                            verse_num = match.group(1)
> +                        else:
> +                            verse_num = '1'
> +                        verse_def = '%s%s' % (verse_tag, verse_num)
> +                    else:
> +                        if parts.endswith('\n'):
> +                            parts = parts.rstrip('\n')
> +                        item = QtWidgets.QTableWidgetItem(parts)
> +                        item.setData(QtCore.Qt.UserRole, verse_def)
> +                        self.verse_list_widget.setRowCount(self.verse_list_widget.rowCount() + 1)
> +                        self.verse_list_widget.setItem(self.verse_list_widget.rowCount() - 1, 0, item)
> +        # Finalize edit
>          self.tag_rows()
>          # Check if all verse tags are used.
>          self.on_verse_order_text_changed(self.verse_order_edit.text())
> 
> === modified file 'openlp/plugins/songs/forms/editversedialog.py'
> --- openlp/plugins/songs/forms/editversedialog.py	2015-12-31 22:46:06 +0000
> +++ openlp/plugins/songs/forms/editversedialog.py	2016-05-09 19:57:28 +0000
> @@ -44,6 +44,10 @@
>          self.split_button.setIcon(build_icon(':/general/general_add.png'))
>          self.split_button.setObjectName('split_button')
>          self.verse_type_layout.addWidget(self.split_button)
> +        self.force_split_button = QtWidgets.QPushButton(edit_verse_dialog)
> +        self.force_split_button.setIcon(build_icon(':/general/general_add.png'))
> +        self.force_split_button.setObjectName('force_split_button')
> +        self.verse_type_layout.addWidget(self.force_split_button)

What do you mean? The UI is the new "Split" button found in song editor, in both "Edit all" and "Edit" modes.

>          self.verse_type_label = QtWidgets.QLabel(edit_verse_dialog)
>          self.verse_type_label.setObjectName('verse_type_label')
>          self.verse_type_layout.addWidget(self.verse_type_label)


-- 
https://code.launchpad.net/~suutari-olli/openlp/force-split/+merge/294159
Your team OpenLP Core is subscribed to branch lp:openlp.