Fix hunk handling for 'default hunk header values'.

patch.py

 # coding=utf8
 # Copyright (c) 2012 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 """Utility functions to handle patches."""
 
 import posixpath
 import os
 import re
 
@@ skipping 117 matching lines @@
   """Parsed hunk data container."""
 
   def __init__(self, start_src, lines_src, start_dst, lines_dst):
     self.start_src = start_src
     self.lines_src = lines_src
     self.start_dst = start_dst
     self.lines_dst = lines_dst
     self.variation = self.lines_dst - self.lines_src
     self.text = []
 
+  def __repr__(self):
+    return '%s<%d, %d, %d, %d>' % (

[Peter Mayo, 2012/08/29 16:49:40]

Question: are there any punctuation/separators that would make this
representation more self-documenting, like -> or -, +, ...
?

[M-A Ruel, 2012/08/29 17:15:29]

Used (%d, %d) to (%d, %d) instead, but that's really just for internal
consumption.

+        self.__class__.__name__,
+        self.start_src, self.lines_src, self.start_dst, self.lines_dst)
+
 
 class FilePatchDiff(FilePatchBase):
   """Patch for a single file."""
 
   def __init__(self, filename, diff, svn_properties):
     super(FilePatchDiff, self).__init__(filename)
     if not diff:
       self._fail('File doesn\'t have a diff.')
     self.diff_header, self.diff_hunks = self._split_header(diff)
     self.svn_properties = svn_properties or []
@@ skipping 75 matching lines @@
     # http://codereview.chromium.org/download/issue6250123_3013_6010.diff
     # Rename no change:
     # http://codereview.chromium.org/download/issue6287022_3001_4010.diff
     return any(l.startswith('diff --git') for l in diff_header.splitlines())
 
   def _split_hunks(self):
     """Splits the hunks and does verification."""
     hunks = []
     for line in self.diff_hunks.splitlines(True):
       if line.startswith('@@'):
         match = re.match(r'^@@ -([\d,]+) \+([\d,]+) @@.*$', line)

[Peter Mayo, 2012/08/29 16:49:40]

This will match multiple commas, which might be better handled as an exception.

[M-A Ruel, 2012/08/29 17:15:29]

Added code to verify this. Broken hunks are rather rare, it's really just to
make sure the parsing code is not receiving unexpected data. Anyhow,
double-commas was already throwing an exception, the error message is simply
nicer now.

         # File add will result in "-0,0 +1" but file deletion will result in
         # "-1,N +0,0" where N is the number of lines deleted. That's from diff
         # and svn diff. git diff doesn't exhibit this behavior.
         # svn diff for a single line file rewrite "@@ -1 +1 @@". Fun.
+        # "@@ -1 +1,N @@" is also valid where N is the length of the new file.
         if not match:
           self._fail('Hunk header is unparsable')
         if ',' in match.group(1):
           start_src, lines_src = map(int, match.group(1).split(',', 1))
         else:
           start_src = int(match.group(1))
-          lines_src = 0
+          lines_src = 1
         if ',' in match.group(2):
           start_dst, lines_dst = map(int, match.group(2).split(',', 1))
         else:
           start_dst = int(match.group(2))
-          lines_dst = 0
+          lines_dst = 1
         new_hunk = Hunk(start_src, lines_src, start_dst, lines_dst)
         if hunks:
           if new_hunk.start_src <= hunks[-1].start_src:
             self._fail('Hunks source lines are not ordered')
           if new_hunk.start_dst <= hunks[-1].start_dst:
             self._fail('Hunks destination lines are not ordered')
         hunks.append(new_hunk)
         continue
       hunks[-1].text.append(line)
 
     if len(hunks) == 1:
       if hunks[0].start_src == 0 and hunks[0].lines_src == 0:
         self.is_new = True
       if hunks[0].start_dst == 0 and hunks[0].lines_dst == 0:
         self.is_delete = True
 
     if self.is_new and self.is_delete:
       self._fail('Hunk header is all 0')
 
     if not self.is_new and not self.is_delete:
       for hunk in hunks:
         variation = (
             len([1 for i in hunk.text if i.startswith('+')]) -
             len([1 for i in hunk.text if i.startswith('-')]))
         if variation != hunk.variation:
           self._fail(
-              'Hunk header is incorrect: %d vs %d' % (
-                variation, hunk.variation))
+              'Hunk header is incorrect: %d vs %d; %r' % (
+                variation, hunk.variation, hunk))
         if not hunk.start_src:
           self._fail(
               'Hunk header start line is incorrect: %d' % hunk.start_src)
         if not hunk.start_dst:
           self._fail(
               'Hunk header start line is incorrect: %d' % hunk.start_dst)
         hunk.start_src -= 1
         hunk.start_dst -= 1
     if self.is_new and hunks:
       hunks[0].start_dst -= 1
@@ skipping 217 matching lines @@
   def __iter__(self):
     for patch in self.patches:
       yield patch
 
   def __getitem__(self, key):
     return self.patches[key]
 
   @property
   def filenames(self):
     return [p.filename for p in self.patches]