diff --git a/reviewboard/diffviewer/parser.py b/reviewboard/diffviewer/parser.py index d8dce37c64db267c81ff8626d88f3320f8b637f8..87a40a5b7e7939061bb943c4b330f6891dca5198 100644 --- a/reviewboard/diffviewer/parser.py +++ b/reviewboard/diffviewer/parser.py @@ -81,10 +81,19 @@ class DiffParser(object): linenum = self.parse_special_header(linenum, info) linenum = self.parse_diff_header(linenum, info) + if info.get('skip', False): + return linenum, None + # If we have enough information to represent a header, build the # file to return. - if 'origFile' in info and 'newFile' in info and \ - 'origInfo' in info and 'newInfo' in info: + if ('origFile' in info and 'newFile' in info and + 'origInfo' in info and 'newInfo' in info): + if linenum < len(self.lines): + linenum = self.parse_after_headers(linenum, info) + + if info.get('skip', False): + return linenum, None + file = File() file.binary = info.get('binary', False) file.deleted = info.get('deleted', False) @@ -156,6 +165,14 @@ class DiffParser(object): return linenum + def parse_after_headers(self, linenu, info): + """Parses data after the diff headers but before the data. + + By default, this does nothing, but a DiffParser subclass can + override to look for special headers before the content. + """ + return linenum + def parse_filename_header(self, s, linenum): if "\t" in s: # There's a \t separating the filename and info. This is the diff --git a/reviewboard/scmtools/svn.py b/reviewboard/scmtools/svn.py index 0f8f8bfa3d83908ae91b45b1abab39039bb72353..8a56ed18f753db18ebc8b941d3ff860f34a5c23a 100644 --- a/reviewboard/scmtools/svn.py +++ b/reviewboard/scmtools/svn.py @@ -431,11 +431,40 @@ class SVNTool(SCMTool): class SVNDiffParser(DiffParser): BINARY_STRING = "Cannot display: file marked as a binary type." - - def __init__(self, data): - DiffParser.__init__(self, data) + PROPERTY_PATH_RE = re.compile(r'Property changes on: (.*)') + + def parse_diff_header(self, linenum, info): + # We're looking for a SVN property change for SVN < 1.7. + # + # There's going to be at least 5 lines left: + # 1) --- (blah) + # 2) +++ (blah) + # 3) Property changes on: + # 4) ----------------------------------------------------- + # 5) Modified: + if (linenum + 4 < len(self.lines) and + self.lines[linenum].startswith('--- (') and + self.lines[linenum + 1].startswith('+++ (') and + self.lines[linenum + 2].startswith('Property changes on:')): + # Subversion diffs with property changes have no really + # parsable format. The content of a property can easily mimic + # the property change headers. So we can't rely upon it, and + # can't easily display it. Instead, skip it, so it at least + # won't break diffs. + info['skip'] = True + linenum += 4 + + return linenum + else: + return super(SVNDiffParser, self).parse_diff_header(linenum, info) def parse_special_header(self, linenum, info): + if (linenum + 1 < len(self.lines) and + self.lines[linenum] == 'Index:'): + # This is an empty Index: line. This might mean we're parsing + # a property change. + return linenum + 2 + linenum = super(SVNDiffParser, self).parse_special_header(linenum, info) if 'index' in info and linenum != len(self.lines): @@ -451,3 +480,25 @@ class SVNDiffParser(DiffParser): info['newInfo'] = '(working copy)' return linenum + + def parse_after_headers(self, linenum, info): + # We're looking for a SVN property change for SVN 1.7+. + # + # This differs from SVN property changes in older versions of SVN + # in a couple ways: + # + # 1) The ---, +++, and Index: lines have actual filenames. + # Because of this, we won't hit the case in parse_diff_header + # above. + # 2) There's an actual section per-property, so we could parse these + # out in a usable form. We'd still need a way to display that + # sanely, though. + if (self.lines[linenum] == '' and + linenum + 2 < len(self.lines) and + self.lines[linenum + 1].startswith('Property changes on:')): + # Skip over the next 3 lines (blank, "Property changes on:", and + # the "__________" divider. + info['skip'] = True + linenum += 3 + + return linenum diff --git a/reviewboard/scmtools/tests.py b/reviewboard/scmtools/tests.py index 282a643aed4e1da7333c02172faa7ac061de6ef3..f76e34df2fa9b31546ee1c1f57b606f5a82be82e 100644 --- a/reviewboard/scmtools/tests.py +++ b/reviewboard/scmtools/tests.py @@ -602,6 +602,67 @@ class SubversionTests(SCMTestCase): file = self.tool.get_file(filename, rev) patch(diff, file, filename) + def test_svn16_property_diff(self): + """Testing parsing SVN 1.6 diff with property changes""" + prop_diff = ( + "Index:\n" + "======================================================" + "=============\n" + "--- (revision 123)\n" + "+++ (working copy)\n" + "Property changes on: .\n" + "______________________________________________________" + "_____________\n" + "Modified: reviewboard:url\n" + "## -1 +1 ##\n" + "-http://reviews.reviewboard.org\n" + "+http://reviews.reviewboard.org\n") + bin_diff = ( + "Index: binfile\n" + "=======================================================" + "============\nCannot display: file marked as a " + "binary type.\nsvn:mime-type = application/octet-stream\n") + diff = prop_diff + bin_diff + + files = self.tool.get_parser(diff).parse() + self.assertEqual(len(files), 1) + self.assertEqual(files[0].origFile, 'binfile') + self.assertTrue(files[0].binary) + + def test_svn17_property_diff(self): + """Testing parsing SVN 1.7+ diff with property changes""" + prop_diff = ( + "Index .:\n" + "======================================================" + "=============\n" + "--- . (revision 123)\n" + "+++ . (working copy)\n" + "\n" + "Property changes on: .\n" + "______________________________________________________" + "_____________\n" + "Modified: reviewboard:url\n" + "## -0,0 +1,3 ##\n" + "-http://reviews.reviewboard.org\n" + "+http://reviews.reviewboard.org\n" + "Added: myprop\n" + "## -0,0 +1 ##\n" + "+Property test.\n") + bin_diff = ( + "Index: binfile\n" + "=======================================================" + "============\nCannot display: file marked as a " + "binary type.\nsvn:mime-type = application/octet-stream\n") + diff = prop_diff + bin_diff + + files = self.tool.get_parser(diff).parse() + print files + print files[0].__dict__ + + self.assertEqual(len(files), 1) + self.assertEqual(files[0].origFile, 'binfile') + self.assertTrue(files[0].binary) + class PerforceTests(SCMTestCase): """Unit tests for perforce.