launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging lp:~wgrant/launchpad/diff-no-newline into lp:launchpad.

Commit message:
Cope with "No newline at end of file" lines in diff emails, which bzrlib merges into the previous line.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~wgrant/launchpad/diff-no-newline/+merge/264254

Cope with "No newline at end of file" lines in diff emails, which bzrlib merges into the previous line. Look for NO_NL in patches.py to see the weird parsing implementation.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/diff-no-newline into lp:launchpad.

=== modified file 'lib/lp/code/mail/codereviewcomment.py'
--- lib/lp/code/mail/codereviewcomment.py	2015-07-09 05:40:01 +0000
+++ lib/lp/code/mail/codereviewcomment.py	2015-07-09 10:28:28 +0000
@@ -244,16 +244,20 @@
                 hunk_lines.extend(format_comment(comment))
                 hunk_comment = True
 
-            for line in hunk.lines:
-                line_count += 1  # inc hunk lines
+            for hunk_line in hunk.lines:
+                # A single HunkLine can actually represent multiple
+                # lines in the "No newline at end of file" case.
+                hunk_line = str(hunk_line)
+                for line in hunk_line.splitlines():
+                    line_count += 1  # inc hunk lines
 
-                #  line is a ContextLine/ReplaceLine
-                hunk_lines.append(u'> %s' % str(line).rstrip('\n').decode(
-                    'utf-8', 'replace'))
-                comment = comments.get(str(line_count))
-                if comment:
-                    hunk_lines.extend(format_comment(comment))
-                    hunk_comment = True
+                    #  line is a ContextLine/ReplaceLine
+                    hunk_lines.append(u'> %s' % line.rstrip('\n').decode(
+                        'utf-8', 'replace'))
+                    comment = comments.get(str(line_count))
+                    if comment:
+                        hunk_lines.extend(format_comment(comment))
+                        hunk_comment = True
 
             # preserve hunks for context if comment in patch header
             if patch_comment or hunk_comment:

=== modified file 'lib/lp/code/mail/tests/test_codereviewcomment.py'
--- lib/lp/code/mail/tests/test_codereviewcomment.py	2015-07-09 03:12:29 +0000
+++ lib/lp/code/mail/tests/test_codereviewcomment.py	2015-07-09 10:28:28 +0000
@@ -404,6 +404,7 @@
         " c\n"
         "+d\n"
         "+e\n"
+        "\\ No newline at end of file\n"
         "\n"
         "=== modified file 'fulango.py'\n"
         "--- fulano.py\t2014-08-26 15:53:34.000000000 -0400\n"
@@ -618,7 +619,7 @@
             self.getSection(comments).splitlines()[4:12])
 
     def test_comment_in_patch_after_linebreak(self):
-        comments = {'31': 'que?'}
+        comments = {'32': 'que?'}
         self.assertEqual(
             map(unicode, [
                 "> ",