Merge ~cjwatson/launchpad:email-utf8-inline-comments into launchpad:master

Proposed by Colin Watson
Status: Merged
Approved by: Colin Watson
Approved revision: e172c36045dc582376276893e0652982208296ae
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: ~cjwatson/launchpad:email-utf8-inline-comments
Merge into: launchpad:master
Diff against target: 62 lines (+8/-7)
2 files modified
lib/lp/code/mail/codereviewcomment.py (+3/-2)
lib/lp/code/mail/tests/test_codereviewcomment.py (+5/-5)
Reviewer Review Type Date Requested Status
Kristian Glass (community) Approve
Review via email: mp+378414@code.launchpad.net

Commit message

Fix emailing of non-ASCII inline comments

Description of the change

I think this regressed as part of my port of codehosting to Breezy. The tests didn't catch it because they were using \u escapes in non-Unicode string literals, which meant that e.g. \u03b4 was interpreted as the byte sequence '\', 'u', '0', '3', 'b', '4' rather than U+03B4.

To post a comment you must log in.
Revision history for this message
Kristian Glass (doismellburning) wrote :

Cripes, that test-data issue is disconcertingly easy to run into!

LGTM

review: Approve
Revision history for this message
Colin Watson (cjwatson) wrote :

Indeed it is! pyflakes didn't spot it. I grepped for other occurrences and didn't find any though.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/lib/lp/code/mail/codereviewcomment.py b/lib/lp/code/mail/codereviewcomment.py
2index 760d923..953c839 100644
3--- a/lib/lp/code/mail/codereviewcomment.py
4+++ b/lib/lp/code/mail/codereviewcomment.py
5@@ -1,4 +1,4 @@
6-# Copyright 2009-2015 Canonical Ltd. This software is licensed under the
7+# Copyright 2009-2020 Canonical Ltd. This software is licensed under the
8 # GNU Affero General Public License version 3 (see the file LICENSE).
9
10 """Email notifications for code review comments."""
11@@ -175,7 +175,8 @@ def format_comment(comment):
12 comment_lines = []
13 if comment is not None:
14 comment_lines.append(b'')
15- comment_lines.extend(comment.splitlines())
16+ comment_lines.extend(
17+ [line.encode('UTF-8') for line in comment.splitlines()])
18 comment_lines.append(b'')
19 return comment_lines
20
21diff --git a/lib/lp/code/mail/tests/test_codereviewcomment.py b/lib/lp/code/mail/tests/test_codereviewcomment.py
22index 2ea2595..930753e 100644
23--- a/lib/lp/code/mail/tests/test_codereviewcomment.py
24+++ b/lib/lp/code/mail/tests/test_codereviewcomment.py
25@@ -1,4 +1,4 @@
26-# Copyright 2009-2015 Canonical Ltd. This software is licensed under the
27+# Copyright 2009-2020 Canonical Ltd. This software is licensed under the
28 # GNU Affero General Public License version 3 (see the file LICENSE).
29
30 """Test CodeReviewComment emailing functionality."""
31@@ -278,7 +278,7 @@ class TestCodeReviewComment(TestCaseWithFactory):
32 See `build_inline_comments_section` tests for formatting details.
33 """
34 comment = self.makeCommentWithInlineComments(
35- inline_comments={'3': 'Is this from Pl\u0060net Earth ?'})
36+ inline_comments={'3': u'Is this from Pl\u00e4net Earth ?'})
37 switch_dbuser(config.IBranchMergeProposalJobSource.dbuser)
38 mailer = CodeReviewCommentMailer.forCreation(comment)
39 commenter = comment.branch_merge_proposal.registrant
40@@ -295,7 +295,7 @@ class TestCodeReviewComment(TestCaseWithFactory):
41 ('> +++ yvo/yc/pbqr/vagresnprf/qvss.cl '
42 '2010-02-02 15:48:56 +0000'),
43 '',
44- 'Is this from Pl\u0060net Earth ?',
45+ u'Is this from Pl\u00e4net Earth ?',
46 '',
47 ]
48 self.assertEqual(expected_lines, ctrl.body.splitlines()[1:10])
49@@ -548,12 +548,12 @@ class TestInlineCommentsSection(testtools.TestCase):
50 def test_single_line_comment(self):
51 # The inline comments are correctly contextualized in the diff.
52 # and prefixed with '>>> '
53- comments = {'4': '\u03b4\u03bf\u03ba\u03b9\u03bc\u03ae'}
54+ comments = {'4': u'\u03b4\u03bf\u03ba\u03b9\u03bc\u03ae'}
55 self.assertEqual(
56 map(unicode, [
57 '> +++ bar.py\t1969-12-31 19:00:00.000000000 -0500',
58 '',
59- '\u03b4\u03bf\u03ba\u03b9\u03bc\u03ae',
60+ u'\u03b4\u03bf\u03ba\u03b9\u03bc\u03ae',
61 '']),
62 self.getSection(comments).splitlines()[7:11])
63

Subscribers

People subscribed via source and target branches

to status/vote changes: