Merge pull request #88 from rtfpessoa/fix-parser

Fix parsing in cases where body lines can be confused with header lines

src/diff-parser.js

@@ -147,6 +147,11 @@
         .replace(/\r\n?/g, '\n')
         .split('\n');
 
+    /* Diff Header */
+    var oldFileNameHeader = '--- ';
+    var newFileNameHeader = '+++ ';
+    var hunkHeaderPrefix = '@@';
+
     /* Diff */
     var oldMode = /^old mode (\d{6})/;
     var newMode = /^new mode (\d{6})/;
@@ -169,7 +174,7 @@
     var combinedNewFile = /^new file mode (\d{6})/;
     var combinedDeletedFile = /^deleted file mode (\d{6}),(\d{6})/;
 
-    diffLines.forEach(function(line) {
+    diffLines.forEach(function(line, lineIndex) {
       // Unmerged paths, and possibly other non-diffable files
       // https://github.com/scottgonzalez/pretty-diff/issues/11
       // Also, remove some useless lines
@@ -177,14 +182,23 @@
         return;
       }
 
-      if (
-        utils.startsWith(line, 'diff') || // Git diffs always start with diff
-        !currentFile || // If we do not have a file yet, we should crete one
+      var prevLine = diffLines[lineIndex - 1];
+      var nxtLine = diffLines[lineIndex + 1];
+      var afterNxtLine = diffLines[lineIndex + 2];
+
+      if (utils.startsWith(line, 'diff')) {
+        startFile();
+        currentFile.isGitDiff = true;
+        return;
+      }
+
+      if (!currentFile || // If we do not have a file yet, we should crete one
         (
-          currentFile && // If we already have some file in progress and
+          !currentFile.isGitDiff && currentFile && // If we already have some file in progress and
           (
-            currentFile.oldName && utils.startsWith(line, '--- ') || // Either we reached a old file identification line
-            currentFile.newName && utils.startsWith(line, '+++ ')  // Or we reached a new file identification line
+            utils.startsWith(line, oldFileNameHeader) && // If we get to an old file path header line
+            // And is followed by the new file path header line and the hunk header line
+            utils.startsWith(nxtLine, newFileNameHeader) && utils.startsWith(afterNxtLine, hunkHeaderPrefix)
           )
         )
       ) {
@@ -193,6 +207,19 @@
 
       var values;
 
+      /*
+       * We need to make sure that we have the three lines of the header.
+       * This avoids cases like the ones described in:
+       *   - https://github.com/rtfpessoa/diff2html/issues/87
+       */
+      if (
+        (utils.startsWith(line, oldFileNameHeader) &&
+        utils.startsWith(nxtLine, newFileNameHeader) && utils.startsWith(afterNxtLine, hunkHeaderPrefix)) ||
+
+        (utils.startsWith(line, newFileNameHeader) &&
+        utils.startsWith(prevLine, oldFileNameHeader) && utils.startsWith(nxtLine, hunkHeaderPrefix))
+      ) {
+
         /*
          * --- Date Timestamp[FractionalSeconds] TimeZone
          * --- 2002-02-21 23:30:39.942229878 -0800
@@ -215,7 +242,9 @@
           return;
         }
 
-      if (currentFile && utils.startsWith(line, '@')) {
+      }
+
+      if (currentFile && utils.startsWith(line, hunkHeaderPrefix)) {
         startBlock(line);
         return;
       }
@@ -231,13 +260,6 @@
         return;
       }
 
-      if (
-        (currentFile && currentFile.blocks.length) ||
-        (currentBlock && currentBlock.lines.length)
-      ) {
-        startFile();
-      }
-
       /*
        * Git diffs provide more information regarding files modes, renames, copies,
        * commits between changes and similarity indexes

src/utils.js

@@ -34,7 +34,7 @@
       return result;
     }
 
-    return str.indexOf(start) === 0;
+    return str && str.indexOf(start) === 0;
   };
 
   Utils.prototype.valueOrEmpty = function(value) {

test/diff-parser-tests.js

@@ -424,25 +424,72 @@ describe('DiffParser', function() {
       assert.deepEqual(linesContent, ['-test', '+test1r', '+test2r']);
     });
 
+    it('should parse unified diff with multiple hunks and files', function() {
+      var diff =
+        '--- sample.js\n' +
+        '+++ sample.js\n' +
+        '@@ -1 +1,2 @@\n' +
+        '-test\n' +
+        '@@ -10 +20,2 @@\n' +
+        '+test\n' +
+        '--- sample1.js\n' +
+        '+++ sample1.js\n' +
+        '@@ -1 +1,2 @@\n' +
+        '+test1';
+
+      var result = DiffParser.generateDiffJson(diff);
+      assert.equal(2, result.length);
+
+      var file1 = result[0];
+      assert.equal(1, file1.addedLines);
+      assert.equal(1, file1.deletedLines);
+      assert.equal('sample.js', file1.oldName);
+      assert.equal('sample.js', file1.newName);
+      assert.equal(2, file1.blocks.length);
+
+      var linesContent1 = file1.blocks[0].lines.map(function(line) {
+        return line.content;
+      });
+      assert.deepEqual(linesContent1, ['-test']);
+
+      var linesContent2 = file1.blocks[1].lines.map(function(line) {
+        return line.content;
+      });
+      assert.deepEqual(linesContent2, ['+test']);
+
+      var file2 = result[1];
+      assert.equal(1, file2.addedLines);
+      assert.equal(0, file2.deletedLines);
+      assert.equal('sample1.js', file2.oldName);
+      assert.equal('sample1.js', file2.newName);
+      assert.equal(1, file2.blocks.length);
+
+      var linesContent = file2.blocks[0].lines.map(function(line) {
+        return line.content;
+      });
+      assert.deepEqual(linesContent, ['+test1']);
+    });
+
     it('should parse diff with --- and +++ in the context lines', function() {
       var diff =
         '--- sample.js\n' +
         '+++ sample.js\n' +
-        '@@ -1,15 +1,12 @@\n' +
+        '@@ -1,8 +1,8 @@\n' +
         ' test\n' +
         ' \n' +
-        '----\n' +
-        '+test\n' +
+        '-- 1\n' +
+        '--- 1\n' +
+        '---- 1\n' +
         ' \n' +
-        ' test\n' +
-        '----\n' +
-        '\\ No newline at end of file';
+        '++ 2\n' +
+        '+++ 2\n' +
+        '++++ 2';
 
       var result = DiffParser.generateDiffJson(diff);
       var file1 = result[0];
       assert.equal(1, result.length);
-      assert.equal(1, file1.addedLines);
-      assert.equal(2, file1.deletedLines);
+      assert.equal(3, file1.addedLines);
+      assert.equal(3, file1.deletedLines);
       assert.equal('sample.js', file1.oldName);
       assert.equal('sample.js', file1.newName);
       assert.equal(1, file1.blocks.length);
@@ -450,7 +497,30 @@ describe('DiffParser', function() {
       var linesContent = file1.blocks[0].lines.map(function(line) {
         return line.content;
       });
-      assert.deepEqual(linesContent, [' test', ' ', '----', '+test', ' ', ' test', '----']);
+      assert.deepEqual(linesContent,
+        [' test', ' ', '-- 1', '--- 1', '---- 1', ' ', '++ 2', '+++ 2', '++++ 2']);
+    });
+
+    it('should parse diff without proper hunk headers', function() {
+      var diff =
+        '--- sample.js\n' +
+        '+++ sample.js\n' +
+        '@@ @@\n' +
+        ' test';
+
+      var result = DiffParser.generateDiffJson(diff);
+      var file1 = result[0];
+      assert.equal(1, result.length);
+      assert.equal(0, file1.addedLines);
+      assert.equal(0, file1.deletedLines);
+      assert.equal('sample.js', file1.oldName);
+      assert.equal('sample.js', file1.newName);
+      assert.equal(1, file1.blocks.length);
+
+      var linesContent = file1.blocks[0].lines.map(function(line) {
+        return line.content;
+      });
+      assert.deepEqual(linesContent, [' test']);
     });
 
   });