launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging lp:~wgrant/launchpad/unicode-diff-comments into lp:launchpad.

Commit message:
Avoid crashing when emailing out inline comments on a non-ASCII diff. We assume that the diff is UTF-8, but that assumption is already made elsewhere.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1319124 in Launchpad itself: "UnicodeDecodeError mailing an inline code review comment"
  https://bugs.launchpad.net/launchpad/+bug/1319124

For more details, see:
https://code.launchpad.net/~wgrant/launchpad/unicode-diff-comments/+merge/219622

Avoid crashing when emailing out inline comments on a non-ASCII diff. We assume that the diff is UTF-8, but that assumption is already made elsewhere.
-- 
https://code.launchpad.net/~wgrant/launchpad/unicode-diff-comments/+merge/219622
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/unicode-diff-comments into lp:launchpad.

=== modified file 'lib/lp/code/mail/codereviewcomment.py'
--- lib/lp/code/mail/codereviewcomment.py	2014-05-12 23:56:01 +0000
+++ lib/lp/code/mail/codereviewcomment.py	2014-05-15 00:23:24 +0000
@@ -174,13 +174,14 @@
     result_lines = []
     diff_lines = diff_text.splitlines()
     for i in range(1, len(diff_lines)):
-        result_lines.append('> {0}'.format(diff_lines[i]))
+        result_lines.append(
+            u'> {0}'.format(diff_lines[i].decode('utf-8', 'replace')))
         comment = comments.get(str(i))
         if comment is not None:
             result_lines.append('')
             result_lines.extend(comment.splitlines())
             result_lines.append('')
 
-    result_text = '\n'.join(result_lines)
+    result_text = u'\n'.join(result_lines)
 
     return '\n\nInline comments:\n\n%s\n\n' % result_text

=== modified file 'lib/lp/code/mail/tests/test_codereviewcomment.py'
--- lib/lp/code/mail/tests/test_codereviewcomment.py	2014-05-13 04:22:49 +0000
+++ lib/lp/code/mail/tests/test_codereviewcomment.py	2014-05-15 00:23:24 +0000
@@ -401,7 +401,7 @@
         "--- bar\t2009-08-26 15:53:34.000000000 -0400\n"
         "+++ bar\t1969-12-31 19:00:00.000000000 -0500\n"
         "@@ -1,3 +0,0 @@\n"
-        "-a\n"
+        "-\xc3\xa5\n"
         "-b\n"
         "-c\n"
         "--- baz\t1969-12-31 19:00:00.000000000 -0500\n"
@@ -445,14 +445,14 @@
     def test_single_line_comment(self):
         # The inline comments are correctly contextualized in the diff.
         # and prefixed with '>>> '
-        comments = {'1': 'Foo'}
+        comments = {'1': u'\u03b4\u03bf\u03ba\u03b9\u03bc\u03ae'}
         self.assertEqual(
             ['> +++ bar\t1969-12-31 19:00:00.000000000 -0500',
              '',
-             'Foo',
+             u'\u03b4\u03bf\u03ba\u03b9\u03bc\u03ae',
              '',
              '> @@ -1,3 +0,0 @@',
-             '> -a'],
+             u'> -\xe5'],
             self.getSection(comments).splitlines()[4:10])
 
     def test_multi_line_comment(self):
@@ -480,16 +480,3 @@
              'Bar',
              ''],
             self.getSection(comments).splitlines()[4:12])
-
-    def test_unicode_comments(self):
-        # inline comments section is unicode and will be
-        # properly encoded for mailling later on.
-        comments = {'1': u'Polui\xe7\xe3o\u00a9 not \uf200 material!'}
-        self.assertEqual(
-            ['> +++ bar\t1969-12-31 19:00:00.000000000 -0500',
-             '',
-             u'Polui\xe7\xe3o\xa9 not \uf200 material!',
-             '',
-             '> @@ -1,3 +0,0 @@',
-             '> -a'],
-            self.getSection(comments).splitlines()[4:10])