← Back to team overview

openlp-core team mailing list archive

Re: [Merge] lp:~thelinuxguy/openlp/fix-newline-bug into lp:openlp

 

Review: Needs Fixing

One small typo, and a couple comments that need changing.

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-16 11:33: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\x0B\x0C\x0E-\x1F\x7F-\x9F]')

\08 should be \x08

>  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-16 11:33:07 +0000
> @@ -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
> +        string = 'something\nelse'
> +        # WHEN: normalize is called
> +        normalized_string = normalize_str(string)
> +        # THEN: string is unchanged
> +        assert normalized_string == string
> +
> +    def test_normalize_str_removes_null_byte(self):
> +        # GIVEN: a string containing newlines

This comment is incorrect

> +        string = 'somet\x00hing'
> +        # WHEN: normalize is called
> +        normalized_string = normalize_str(string)
> +        # THEN: string is unchanged

so is this

> +        assert normalized_string == 'something'
> +
> +    def test_normalize_str_replaces_crlf_with_lf(self):
> +        # GIVEN: a string containing crlf
> +        string = 'something\r\nelse'
> +        # WHEN: normalize is called
> +        normalized_string = normalize_str(string)
> +        # 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/343306
Your team OpenLP Core is subscribed to branch lp:openlp.


Follow ups

References