openlp-core team mailing list archive

[Tim Bentley <tim.bentley@xxxxxxxxx>]

Review: Needs Fixing

Should be built on top of the websockets branch as the remote is very different.

Looks good and at a first read not many issues.

the move to websockets will move stuff around so will a full review then.

Diff comments:

> === modified file 'openlp/core/lib/__init__.py'
> --- openlp/core/lib/__init__.py	2016-10-30 08:29:22 +0000
> +++ openlp/core/lib/__init__.py	2016-11-16 19:47:28 +0000
> @@ -281,11 +283,12 @@
>      return True
>  
>  
> -def clean_tags(text):
> +def clean_tags(text, remove_chords=False):
>      """

This should be moved to common as it is not a lib feature but common to all the code.
common __init__ will get very bit so need a better structure there?

>      Remove Tags from text for display
>  
>      :param text: Text to be cleaned
> +    :param remove_chords: Clean ChordPro tags
>      """
>      text = text.replace('<br>', '\n')
>      text = text.replace('{br}', '\n')
> @@ -302,12 +308,107 @@
>  
>      :param text: The text to be expanded.
>      """
> +    text = expand_chords(text)
>      for tag in FormattingTags.get_html_tags():
>          text = text.replace(tag['start tag'], tag['start html'])
>          text = text.replace(tag['end tag'], tag['end html'])
>      return text
>  
>  
> +def expand_and_align_chords_in_line(match):
> +    """
> +    Expand the chords in the line and align them using whitespaces.
> +    NOTE: There is equivalent javascript code in chords.js, in the updateSlide function. Make sure to update both!
> +
> +    :param match:
> +    :return: The line with expanded html-chords
> +    """
> +    slimchars = 'fiíIÍjlĺľrtť.,;/ ()|"\'!:\\'

should the a STATIC

> +    whitespaces = ''
> +    chordlen = 0
> +    taillen = 0
> +    # The match could be "[G]sweet the " from a line like "A[D]mazing [D7]grace! How [G]sweet the [D]sound!"
> +    # The actual chord, would be "G" in match "[G]sweet the "
> +    chord = match.group(1)
> +    # The tailing word of the chord, would be "sweet" in match "[G]sweet the "
> +    tail = match.group(2)
> +    # The remainder of the line, until line end or next chord. Would be " the " in match "[G]sweet the "
> +    remainder = match.group(3)
> +    # Line end if found, else None
> +    end = match.group(4)
> +    # Based on char width calculate width of chord
> +    for chord_char in chord:
> +        if chord_char not in slimchars:
> +            chordlen += 2
> +        else:
> +            chordlen += 1
> +    # Based on char width calculate width of tail
> +    for tail_char in tail:
> +        if tail_char not in slimchars:
> +            taillen += 2
> +        else:
> +            taillen += 1
> +    # Based on char width calculate width of remainder
> +    for remainder_char in remainder:
> +        if remainder_char not in slimchars:
> +            taillen += 2
> +        else:
> +            taillen += 1
> +    # If the chord is wider than the tail+remainder and the line goes on, some padding is needed
> +    if chordlen >= taillen and end is None:
> +        # Decide if the padding should be "_" for drawing out words or spaces
> +        if tail:
> +            if not remainder:
> +                for c in range(math.ceil((chordlen - taillen) / 2) + 1):
> +                    whitespaces += '_'
> +            else:
> +                for c in range(chordlen - taillen + 2):
> +                    whitespaces += '&nbsp;'
> +        else:
> +            if not remainder:
> +                for c in range(math.floor((chordlen - taillen) / 2)):
> +                    whitespaces += '_'
> +            else:
> +                for c in range(chordlen - taillen + 1):
> +                    whitespaces += '&nbsp;'
> +    else:
> +        if not tail and remainder and remainder[0] == ' ':
> +            for c in range(chordlen):
> +                whitespaces += '&nbsp;'
> +    if whitespaces:
> +        whitespaces = '<span class="ws">' + whitespaces + '</span>'
> +    return '<span class="chord"><span><strong>' + chord + '</strong></span></span>' + tail + whitespaces + remainder
> +
> +
> +def expand_chords(text):
> +    """
> +    Expand ChordPro tags
> +
> +    :param text:
> +    """
> +    text_lines = text.split('{br}')
> +    expanded_text_lines = []
> +    chords_on_prev_line = False
> +    for line in text_lines:
> +        # If a ChordPro is detected in the line, replace it with a html-span tag and wrap the line in a span tag.
> +        if '[' in line and ']' in line:
> +            if chords_on_prev_line:
> +                new_line = '<span class="chordline">'
> +            else:
> +                new_line = '<span class="chordline firstchordline">'
> +                chords_on_prev_line = True
> +            # Matches a chord, a tail, a remainder and a line end. See expand_and_align_chords_in_line() for more info.
> +            new_line += re.sub(r'\[(\w.*?)\]([\u0080-\uFFFF,\w]*)'
> +                               '([\u0080-\uFFFF,\w,\s,\.,\,,\!,\?,\;,\:,\|,\",\',\-,\_]*)(\Z)?',
> +                               expand_and_align_chords_in_line, line)
> +            new_line += '</span>'
> +            expanded_text_lines.append(new_line)
> +        else:
> +            chords_on_prev_line = False
> +            expanded_text_lines.append(line)
> +    return '{br}'.join(expanded_text_lines)
> +
> +
>  def create_separated_list(string_list):
>      """
>      Returns a string that represents a join of a list of strings with a localized separator. This function corresponds
> 
> === modified file 'openlp/plugins/songs/lib/songstab.py'
> --- openlp/plugins/songs/lib/songstab.py	2016-07-24 20:20:25 +0000
> +++ openlp/plugins/songs/lib/songstab.py	2016-11-16 19:47:28 +0000
> @@ -57,6 +57,34 @@
>          self.display_copyright_check_box.setObjectName('copyright_check_box')
>          self.mode_layout.addWidget(self.display_copyright_check_box)
>          self.left_layout.addWidget(self.mode_group_box)
> +
> +        self.chords_group_box = QtWidgets.QGroupBox(self.left_column)
> +        self.chords_group_box.setObjectName('chords_group_box')
> +        self.chords_layout = QtWidgets.QVBoxLayout(self.chords_group_box)
> +        self.chords_layout.setObjectName('chords_layout')
> +        self.mainview_chords_check_box = QtWidgets.QCheckBox(self.mode_group_box)
> +        self.mainview_chords_check_box.setObjectName('tool_bar_active_check_box')
> +        self.chords_layout.addWidget(self.mainview_chords_check_box)
> +        self.disable_chords_import_check_box = QtWidgets.QCheckBox(self.mode_group_box)
> +        self.disable_chords_import_check_box.setObjectName('tool_bar_active_check_box')
> +        self.chords_layout.addWidget(self.disable_chords_import_check_box)
> +

Blank lines

> +        # Chords notation
> +        self.chord_notation_label = QtWidgets.QLabel(self.chords_group_box)
> +        self.chord_notation_label.setWordWrap(True)
> +        self.chords_layout.addWidget(self.chord_notation_label)
> +        self.english_notation_radio_button = QtWidgets.QRadioButton(self.chords_group_box)
> +        self.english_notation_radio_button.setObjectName('english_notation_radio_button')
> +        self.chords_layout.addWidget(self.english_notation_radio_button)
> +        self.german_notation_radio_button = QtWidgets.QRadioButton(self.chords_group_box)
> +        self.german_notation_radio_button.setObjectName('german_notation_radio_button')
> +        self.chords_layout.addWidget(self.german_notation_radio_button)
> +        self.neolatin_notation_radio_button = QtWidgets.QRadioButton(self.chords_group_box)
> +        self.neolatin_notation_radio_button.setObjectName('neolatin_notation_radio_button')
> +        self.chords_layout.addWidget(self.neolatin_notation_radio_button)
> +
> +        self.left_layout.addWidget(self.chords_group_box)
> +
>          self.left_layout.addStretch()
>          self.right_layout.addStretch()
>          self.tool_bar_active_check_box.stateChanged.connect(self.on_tool_bar_active_check_box_changed)


-- 
https://code.launchpad.net/~tomasgroth/openlp/chords/+merge/311066
Your team OpenLP Core is subscribed to branch lp:openlp.