Re: [Openlp-core] [Merge] lp:~thelinuxguy/openlp/fix-newline-bug into lp:openlp
> One small typo, and a couple comments that need changing. Sorry! -- https://code.launchpad.net/~thelinuxguy/openlp/fix-newline-bug/+merge/343306 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
Re: [Openlp-core] [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__.py2018-02-24 16:10:02 +
> +++ openlp/core/common/__init__.py2018-04-16 11:33:07 +
> @@ -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.py2017-12-29
> 09:15:48 +
> +++ tests/functional/openlp_core/common/test_common.py2018-04-16
> 11:33:07 +
> @@ -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.
___
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
[Openlp-core] [Merge] lp:~thelinuxguy/openlp/fix-newline-bug into lp:openlp
The proposal to merge lp:~thelinuxguy/openlp/fix-newline-bug into lp:openlp has been updated. Status: Needs review => Superseded For more details, see: https://code.launchpad.net/~thelinuxguy/openlp/fix-newline-bug/+merge/343277 -- 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
Re: [Openlp-core] [Merge] lp:~thelinuxguy/openlp/fix-newline-bug into lp:openlp
I wonder what the reason behind replacing vertial tabs with new lines is... Looking at where the regex is being used I'm inclined to say the whole thing needs to be reworked. It's also used in the filename cleaning where linefeeds are strange, but then again I don't know what the input always is. I'm changing it to what you request, but you should really take a serious look at it. From what I see, you are the last one that touched the invalid file regex, I highly doubt this is what you actually want to happen. If it is, it is a mystery to me... -- https://code.launchpad.net/~thelinuxguy/openlp/fix-newline-bug/+merge/343277 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
Re: [Openlp-core] [Merge] lp:~thelinuxguy/openlp/fix-newline-bug into lp:openlp
Review: Needs Fixing
I've done some research in to this. Officially the only code points allowed are:
"
[2] Char ::= #x9 | #xA | #xD | [#x20-#xD7FF] |
[#xE000-#xFFFD] | [#x1-#x10] /* any Unicode character, excluding the
surrogate blocks, FFFE, and . */
" ( https://www.w3.org/TR/REC-xml/#charsets)
So going on this our regex should be:
re.compile(r'[\x00-\x08\x0B\x0C\x0E-\x1F\x7F-\x9F]')
we probably should filter out the other code points too.
Its worth noting that REPLACMENT_CHARS_MAP replaces the vertical tab and form
feed chars with "\n\n" before the CONTROL_CHARS regex filters them out!
So in summary please replace the regex with the one in the inline comment.
(Note: its not tested)
Diff comments:
> === modified file 'openlp/core/common/__init__.py'
> --- openlp/core/common/__init__.py2018-02-24 16:10:02 +
> +++ openlp/core/common/__init__.py2018-04-15 14:10:22 +
> @@ -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]')
CONTROL_CHARS = re.compile(r'[\x00-\x08\x0B\x0C\x0E-\x1F\x7F-\x9F]')
> INVALID_FILE_CHARS = re.compile(r'[\\/:\*\?"<>\|\+\[\]%]')
> IMAGES_FILTER = None
> REPLACMENT_CHARS_MAP = str.maketrans({'\u2018': '\'', '\u2019': '\'',
> '\u201c': '"', '\u201d': '"', '\u2026': '...',
--
https://code.launchpad.net/~thelinuxguy/openlp/fix-newline-bug/+merge/343277
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
