Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Issue 5172705124941824: Issue 2452 - Avoid replacing "\n" with newline in fixed strings (Closed)

Created:
May 6, 2015, 4 p.m. by kzar
Modified:
May 6, 2015, 4:50 p.m.
Visibility:
Public.

Description

Issue 2452 - Avoid replacing "\n" with newline in fixed strings

Patch Set 1 #

Total comments: 3

Patch Set 2 : Tidied up loop #

Unified diffs Side-by-side diffs Delta from patch set Stats (+2 lines, -2 lines) Patch
M cms/converters.py View 1 1 chunk +2 lines, -2 lines 0 comments Download

Messages

Total messages: 6
kzar
May 6, 2015, 4:01 p.m. (2015-05-06 16:01:38 UTC) #1
Sebastian Noack
http://codereview.adblockplus.org/5172705124941824/diff/5629499534213120/cms/converters.py File cms/converters.py (left): http://codereview.adblockplus.org/5172705124941824/diff/5629499534213120/cms/converters.py#oldcode145 cms/converters.py:145: for i in range(len(fixed_strings)): While on it, mind making ...
May 6, 2015, 4:24 p.m. (2015-05-06 16:24:30 UTC) #2
Sebastian Noack
http://codereview.adblockplus.org/5172705124941824/diff/5629499534213120/cms/converters.py File cms/converters.py (left): http://codereview.adblockplus.org/5172705124941824/diff/5629499534213120/cms/converters.py#oldcode145 cms/converters.py:145: for i in range(len(fixed_strings)): On 2015/05/06 16:24:30, Sebastian Noack ...
May 6, 2015, 4:25 p.m. (2015-05-06 16:25:23 UTC) #3
kzar
Patch Set 2 : Tidied up loop http://codereview.adblockplus.org/5172705124941824/diff/5629499534213120/cms/converters.py File cms/converters.py (left): http://codereview.adblockplus.org/5172705124941824/diff/5629499534213120/cms/converters.py#oldcode145 cms/converters.py:145: for i ...
May 6, 2015, 4:30 p.m. (2015-05-06 16:30:41 UTC) #4
Sebastian Noack
LGTM
May 6, 2015, 4:31 p.m. (2015-05-06 16:31:52 UTC) #5
Wladimir Palant
May 6, 2015, 4:40 p.m. (2015-05-06 16:40:49 UTC) #6
LGTM

We might want to look at other re.sub() calls to ensure that there is no issue
there.

Powered by Google App Engine
This is Rietveld