launchpad-reviewers team mailing list archive

[Celso Providelo <celso.providelo@xxxxxxxxxxxxx>]

Review: Approve

Thanks for fixing my mistakes :-) Looks good for landing.

Inline comments:

> --- 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))

Duh, this but was indeed my fault. I didn't realize I had to decode our UTF-8 diffs.
Thanks for spotting it.

>          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])
> 

Thanks for collapsing the tests.



-- 
https://code.launchpad.net/~wgrant/launchpad/unicode-diff-comments/+merge/219622
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.