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
diff --git a/lib/lp/code/mail/codereviewcomment.py b/lib/lp/code/mail/codereviewcomment.py
index 760d923..953c839 100644
--- a/lib/lp/code/mail/codereviewcomment.py
+++ b/lib/lp/code/mail/codereviewcomment.py
@@ -1,4 +1,4 @@
1# Copyright 2009-2015 Canonical Ltd. This software is licensed under the1# Copyright 2009-2020 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).2# GNU Affero General Public License version 3 (see the file LICENSE).
33
4"""Email notifications for code review comments."""4"""Email notifications for code review comments."""
@@ -175,7 +175,8 @@ def format_comment(comment):
175 comment_lines = []175 comment_lines = []
176 if comment is not None:176 if comment is not None:
177 comment_lines.append(b'')177 comment_lines.append(b'')
178 comment_lines.extend(comment.splitlines())178 comment_lines.extend(
179 [line.encode('UTF-8') for line in comment.splitlines()])
179 comment_lines.append(b'')180 comment_lines.append(b'')
180 return comment_lines181 return comment_lines
181182
diff --git a/lib/lp/code/mail/tests/test_codereviewcomment.py b/lib/lp/code/mail/tests/test_codereviewcomment.py
index 2ea2595..930753e 100644
--- a/lib/lp/code/mail/tests/test_codereviewcomment.py
+++ b/lib/lp/code/mail/tests/test_codereviewcomment.py
@@ -1,4 +1,4 @@
1# Copyright 2009-2015 Canonical Ltd. This software is licensed under the1# Copyright 2009-2020 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).2# GNU Affero General Public License version 3 (see the file LICENSE).
33
4"""Test CodeReviewComment emailing functionality."""4"""Test CodeReviewComment emailing functionality."""
@@ -278,7 +278,7 @@ class TestCodeReviewComment(TestCaseWithFactory):
278 See `build_inline_comments_section` tests for formatting details.278 See `build_inline_comments_section` tests for formatting details.
279 """279 """
280 comment = self.makeCommentWithInlineComments(280 comment = self.makeCommentWithInlineComments(
281 inline_comments={'3': 'Is this from Pl\u0060net Earth ?'})281 inline_comments={'3': u'Is this from Pl\u00e4net Earth ?'})
282 switch_dbuser(config.IBranchMergeProposalJobSource.dbuser)282 switch_dbuser(config.IBranchMergeProposalJobSource.dbuser)
283 mailer = CodeReviewCommentMailer.forCreation(comment)283 mailer = CodeReviewCommentMailer.forCreation(comment)
284 commenter = comment.branch_merge_proposal.registrant284 commenter = comment.branch_merge_proposal.registrant
@@ -295,7 +295,7 @@ class TestCodeReviewComment(TestCaseWithFactory):
295 ('> +++ yvo/yc/pbqr/vagresnprf/qvss.cl '295 ('> +++ yvo/yc/pbqr/vagresnprf/qvss.cl '
296 '2010-02-02 15:48:56 +0000'),296 '2010-02-02 15:48:56 +0000'),
297 '',297 '',
298 'Is this from Pl\u0060net Earth ?',298 u'Is this from Pl\u00e4net Earth ?',
299 '',299 '',
300 ]300 ]
301 self.assertEqual(expected_lines, ctrl.body.splitlines()[1:10])301 self.assertEqual(expected_lines, ctrl.body.splitlines()[1:10])
@@ -548,12 +548,12 @@ class TestInlineCommentsSection(testtools.TestCase):
548 def test_single_line_comment(self):548 def test_single_line_comment(self):
549 # The inline comments are correctly contextualized in the diff.549 # The inline comments are correctly contextualized in the diff.
550 # and prefixed with '>>> '550 # and prefixed with '>>> '
551 comments = {'4': '\u03b4\u03bf\u03ba\u03b9\u03bc\u03ae'}551 comments = {'4': u'\u03b4\u03bf\u03ba\u03b9\u03bc\u03ae'}
552 self.assertEqual(552 self.assertEqual(
553 map(unicode, [553 map(unicode, [
554 '> +++ bar.py\t1969-12-31 19:00:00.000000000 -0500',554 '> +++ bar.py\t1969-12-31 19:00:00.000000000 -0500',
555 '',555 '',
556 '\u03b4\u03bf\u03ba\u03b9\u03bc\u03ae',556 u'\u03b4\u03bf\u03ba\u03b9\u03bc\u03ae',
557 '']),557 '']),
558 self.getSection(comments).splitlines()[7:11])558 self.getSection(comments).splitlines()[7:11])
559559

Subscribers

People subscribed via source and target branches

to status/vote changes: