From ed8d33930df2ad3f5ecf652b28b9c01091239dde Mon Sep 17 00:00:00 2001 From: Rodrigo Fernandes Date: Fri, 2 Sep 2016 17:39:52 +0100 Subject: [PATCH] Fix git diff with renames and hunk headers When we have the file names we consider that the file is already past the hunk header (@@) and since the renames were setting the names before the diff was confused. To fix this I kind of hacked a solution that only uses the rename file paths if there is no hunk header comming. We need to find a better way to decide when to start new hunk, finish files etc --- src/diff-parser.js | 65 ++++++++++++++++++++++++++++++--------- test/diff-parser-tests.js | 32 +++++++++++++++++++ 2 files changed, 83 insertions(+), 14 deletions(-) diff --git a/src/diff-parser.js b/src/diff-parser.js index e899f1e..7a81895 100644 --- a/src/diff-parser.js +++ b/src/diff-parser.js @@ -38,26 +38,26 @@ var hunkHeaderPrefix = '@@'; /* Add previous block(if exists) before start a new file */ - var saveBlock = function() { + function saveBlock() { if (currentBlock) { currentFile.blocks.push(currentBlock); currentBlock = null; } - }; + } /* * Add previous file(if exists) before start a new one * if it has name (to avoid binary files errors) */ - var saveFile = function() { + function saveFile() { if (currentFile && currentFile.newName) { files.push(currentFile); currentFile = null; } - }; + } /* Create file structure */ - var startFile = function() { + function startFile() { saveBlock(); saveFile(); @@ -65,9 +65,9 @@ currentFile.blocks = []; currentFile.deletedLines = 0; currentFile.addedLines = 0; - }; + } - var startBlock = function(line) { + function startBlock(line) { saveBlock(); var values; @@ -112,9 +112,9 @@ currentBlock.oldStartLine2 = oldLine2; currentBlock.newStartLine = newLine; currentBlock.header = line; - }; + } - var createLine = function(line) { + function createLine(line) { var currentLine = {}; currentLine.content = line; @@ -145,7 +145,34 @@ currentBlock.lines.push(currentLine); } - }; + } + + /* + * Checks if there is a hunk header coming before a new file starts + * + * Hunk header is a group of three lines started by ( `--- ` , `+++ ` , `@@` ) + */ + function existHunkHeader(line, lineIdx) { + var idx = lineIdx; + + while (idx < diffLines.length - 3) { + if (utils.startsWith(line, 'diff')) { + return false; + } + + if ( + utils.startsWith(diffLines[idx], oldFileNameHeader) && + utils.startsWith(diffLines[idx + 1], newFileNameHeader) && + utils.startsWith(diffLines[idx + 2], hunkHeaderPrefix) + ) { + return true; + } + + idx++; + } + + return false; + } var diffLines = diffInput.replace(/\\ No newline at end of file/g, '') @@ -261,6 +288,8 @@ return; } + var doesNotExistHunkHeader = !existHunkHeader(line, lineIndex); + /* * Git diffs provide more information regarding files modes, renames, copies, * commits between changes and similarity indexes @@ -276,16 +305,24 @@ currentFile.newFileMode = values[1]; currentFile.isNew = true; } else if ((values = copyFrom.exec(line))) { - currentFile.oldName = values[1]; + if (doesNotExistHunkHeader) { + currentFile.oldName = values[1]; + } currentFile.isCopy = true; } else if ((values = copyTo.exec(line))) { - currentFile.newName = values[1]; + if (doesNotExistHunkHeader) { + currentFile.newName = values[1]; + } currentFile.isCopy = true; } else if ((values = renameFrom.exec(line))) { - currentFile.oldName = values[1]; + if (doesNotExistHunkHeader) { + currentFile.oldName = values[1]; + } currentFile.isRename = true; } else if ((values = renameTo.exec(line))) { - currentFile.newName = values[1]; + if (doesNotExistHunkHeader) { + currentFile.newName = values[1]; + } currentFile.isRename = true; } else if ((values = similarityIndex.exec(line))) { currentFile.unchangedPercentage = values[1]; diff --git a/test/diff-parser-tests.js b/test/diff-parser-tests.js index 682abce..95f6155 100644 --- a/test/diff-parser-tests.js +++ b/test/diff-parser-tests.js @@ -543,5 +543,37 @@ describe('DiffParser', function() { assert.equal('Binary files differ', file1.blocks[0].header); }); + it('should parse diff with --find-renames', function() { + var diff = + 'diff --git a/src/test-bar.js b/src/test-baz.js\n' + + 'similarity index 98%\n' + + 'rename from src/test-bar.js\n' + + 'rename to src/test-baz.js\n' + + 'index e01513b..f14a870 100644\n' + + '--- a/src/test-bar.js\n' + + '+++ b/src/test-baz.js\n' + + '@@ -1,4 +1,32 @@\n' + + ' function foo() {\n' + + '-var bar = "Whoops!";\n' + + '+var baz = "Whoops!";\n' + + ' }\n' + + ' '; + + var result = DiffParser.generateDiffJson(diff); + var file1 = result[0]; + assert.equal(1, result.length); + assert.equal(1, file1.addedLines); + assert.equal(1, file1.deletedLines); + assert.equal('src/test-bar.js', file1.oldName); + assert.equal('src/test-baz.js', file1.newName); + assert.equal(1, file1.blocks.length); + assert.equal(5, file1.blocks[0].lines.length); + var linesContent = file1.blocks[0].lines.map(function(line) { + return line.content; + }); + assert.deepEqual(linesContent, + [' function foo() {', '-var bar = "Whoops!";', '+var baz = "Whoops!";', ' }', ' ']); + }); + }); });