Review: Needs Fixing
See in line. One question and a few (more) minor fixes please.
Diff comments:
> === modified file 'openlp/core/common/__init__.py'
> --- openlp/core/common/__init__.py 2018-02-24 16:10:02 +0000
> +++ openlp/core/common/__init__.py 2018-04-14 20:05:07 +0000
> @@ -44,7 +44,7 @@
>
> FIRST_CAMEL_REGEX = re.compile('(.)([A-Z][a-z]+)')
> SECOND_CAMEL_REGEX = re.compile('([a-z0-9])([A-Z])')
> -CONTROL_CHARS = re.compile(r'[\x00-\x1F\x7F-\x9F]')
> +CONTROL_CHARS = re.compile(r'[\x00-\08\x0E-\x1F\x7F-\x9F]')
is there any reason for removing 09, 0B, 0C (horizontal and vertical tabs as
well as form feed)?
> INVALID_FILE_CHARS = re.compile(r'[\\/:\*\?"<>\|\+\[\]%]')
> IMAGES_FILTER = None
> REPLACMENT_CHARS_MAP = str.maketrans({'\u2018': '\'', '\u2019': '\'',
> '\u201c': '"', '\u201d': '"', '\u2026': '...',
>
> === modified file 'tests/functional/openlp_core/common/test_common.py'
> --- tests/functional/openlp_core/common/test_common.py 2017-12-29
> 09:15:48 +0000
> +++ tests/functional/openlp_core/common/test_common.py 2018-04-14
> 20:05:07 +0000
> @@ -25,8 +25,8 @@
> from unittest import TestCase
> from unittest.mock import MagicMock, call, patch
>
> -from openlp.core.common import clean_button_text, de_hump, extension_loader,
> is_macosx, is_linux, is_win, \
> - path_to_module, trace_error_handler
> +from openlp.core.common import (clean_button_text, de_hump,
> extension_loader, is_macosx, is_linux,
> + is_win, normalize_str, path_to_module,
> trace_error_handler)
can you remove the brackets from the import statement please, its not our style.
> from openlp.core.common.path import Path
>
>
> @@ -211,6 +211,30 @@
> assert is_win() is False, 'is_win() should return False'
> assert is_macosx() is False, 'is_macosx() should return False'
>
> + def test_normalize_str_leaves_newlines(self):
> + # GIVEN: a string containing newlines
> + str = 'something\nelse'
str is a built-in function, please use another name
> + # WHEN: normalize is called
> + normalized_string = normalize_str(str)
> + # THEN: string is unchanged
> + assert normalized_string == str
> +
> + def test_normalize_str_removes_null_byte(self):
> + # GIVEN: a string containing newlines
> + str = 'somet\x00hing'
and here
> + # WHEN: normalize is called
> + normalized_string = normalize_str(str)
> + # THEN: string is unchanged
> + assert normalized_string == 'something'
> +
> + def test_normalize_str_replaces_crlf_with_lf(self):
> + # GIVEN: a string containing crlf
> + str = 'something\r\nelse'
here too!
> + # WHEN: normalize is called
> + normalized_string = normalize_str(str)
> + # THEN: crlf is replaced with lf
> + assert normalized_string == 'something\nelse'
> +
> def test_clean_button_text(self):
> """
> Test the clean_button_text() function.
--
https://code.launchpad.net/~thelinuxguy/openlp/fix-newline-bug/+merge/343256
Your team OpenLP Core is subscribed to branch lp:openlp.
_______________________________________________
Mailing list: https://launchpad.net/~openlp-core
Post to : openlp-core@lists.launchpad.net
Unsubscribe : https://launchpad.net/~openlp-core
More help : https://help.launchpad.net/ListHelp
