Merge lp:~cjwatson/launchpad/git-patch-headers into lp:launchpad

[Colin Watson]

Parse extended header lines in git diffs correctly.

Parse extended header lines in git diffs correctly.

https://git.kernel.org/cgit/git/git.git/tree/Documentation/diff-generate-patch.txt documents the file format here. Rather than either (a) hardcoding the longish list of possible extended header lines or (b) being more liberal with all "dirty header" lines and risking disturbing parsing of Bazaar diffs, I opted for (c) instead, namely to accept anything starting with a lower-case letter as an extended header line provided that it hasn't already matched one of dirty_headers, but only if it's after a line starting with "diff --git".

[Kit Randel (blr)]

That seems like a reasonably future-proof approach.

review: Approve

=== modified file 'lib/lp/code/mail/patches.py'
--- lib/lp/code/mail/patches.py	2015-07-09 05:40:01 +0000
+++ lib/lp/code/mail/patches.py	2015-10-27 13:03:57 +0000
@@ -360,6 +360,7 @@
     dirty_head = []
     orig_range = 0
     beginning = True
+    in_git_patch = False
 
     dirty_headers = ('=== ', 'diff ', 'index ')
     for line in iter_lines:
@@ -372,7 +373,16 @@
                     dirty_head = []
                 else:
                     yield saved_lines
+                in_git_patch = False
                 saved_lines = []
+            if line.startswith('diff --git'):
+                in_git_patch = True
+            dirty_head.append(line)
+            continue
+        if in_git_patch and line and line[0].islower():
+            # Extended header line in a git diff.  All extended header lines
+            # currently start with a lower-case character, and nothing else
+            # in the patch before the next "diff" header line can do so.
             dirty_head.append(line)
             continue
         if line.startswith('*** '):
@@ -395,6 +405,7 @@
                     dirty_head = []
                 else:
                     yield saved_lines
+                in_git_patch = False
             saved_lines = []
         elif line.startswith('@@'):
             hunk = hunk_from_header(line)

=== modified file 'lib/lp/code/mail/tests/test_codereviewcomment.py'
--- lib/lp/code/mail/tests/test_codereviewcomment.py	2015-09-11 12:20:23 +0000
+++ lib/lp/code/mail/tests/test_codereviewcomment.py	2015-10-27 13:03:57 +0000
@@ -468,7 +468,9 @@
         "-bar\n"
         "+baz\n"
         "diff --git a/fulano b/fulano\n"
-        "index 5716ca5..7601807 100644\n"
+        "old mode 100644\n"
+        "new mode 100755\n"
+        "index 5716ca5..7601807\n"
         "--- a/fulano\n"
         "+++ b/fulano\n"
         "@@ -1,3 +1,3 @@\n"
@@ -556,7 +558,7 @@
             self.getSection(comments).splitlines()[7:11])
 
     def test_comments_in_git_diff(self):
-        comments = {'1': 'foo', '5': 'bar', '15': 'baz'}
+        comments = {'1': 'foo', '5': 'bar', '17': 'baz'}
         section = self.getSection(comments, diff_text=self.git_diff_text)
         self.assertEqual(
             map(unicode, [
@@ -574,7 +576,9 @@
                 "> -bar",
                 "> +baz",
                 "> diff --git a/fulano b/fulano",
-                "> index 5716ca5..7601807 100644",
+                "> old mode 100644",
+                "> new mode 100755",
+                "> index 5716ca5..7601807",
                 "> --- a/fulano",
                 "> +++ b/fulano",
                 "> @@ -1,3 +1,3 @@",
@@ -585,7 +589,7 @@
                 "baz",
                 "",
                 "> +zutano"]),
-            section.splitlines()[4:29])
+            section.splitlines()[4:31])
 
     def test_commentless_hunks_ignored(self):
         # Hunks without inline comments are not returned in the diff text.
@@ -694,7 +698,7 @@
             self.getSection(comments).splitlines()[6:12])
 
     def test_multiple_comments(self):
-        # Multiple inline comments are redered appropriately.
+        # Multiple inline comments are rendered appropriately.
         comments = {'4': 'Foo', '5': 'Bar'}
         self.assertEqual(
             ['> +++ bar.py\t1969-12-31 19:00:00.000000000 -0500',