8000 Merge pull request #88 from rtfpessoa/fix-parser · hubgit/diff2html@8679bc3 · GitHub
[go: up one dir, main page]
Skip to content

Commit 8679bc3

Browse files
rtfpessoartfpessoa
authored
Merge pull request rtfpessoa#88 from rtfpessoa/fix-parser
Fix parsing in cases where body lines can be confused with header lines
2 parents 5e0ef64 + f2858f6 commit 8679bc3

File tree

3 files changed

+134
-42
lines changed

3 files changed

+134
-42
lines changed

‎src/diff-parser.js

Lines changed: 54 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,11 @@
147147
.replace(/\r\n?/g, '\n')
148148
.split('\n');
149149

150+
/* Diff Header */
151+
var oldFileNameHeader = '--- ';
152+
var newFileNameHeader = '+++ ';
153+
var hunkHeaderPrefix = '@@';
154+
150155
/* Diff */
151156
var oldMode = /^old mode (\d{6})/;
152157
var newMode = /^new mode (\d{6})/;
@@ -169,22 +174,31 @@
169174
var combinedNewFile = /^new file mode (\d{6})/;
170175
var combinedDeletedFile = /^deleted file mode (\d{6}),(\d{6})/;
171176

172-
diffLines.forEach(function(line) {
177+
diffLines.forEach(function(line, lineIndex) {
173178
// Unmerged paths, and possibly other non-diffable files
174179
// https://github.com/scottgonzalez/pretty-diff/issues/11
175180
// Also, remove some useless lines
176181
if (!line || utils.startsWith(line, '*')) {
177182
return;
178183
}
179184

180-
if (
181-
utils.startsWith(line, 'diff') || // Git diffs always start with diff
182-
!currentFile || // If we do not have a file yet, we should crete one
185+
var prevLine = diffLines[lineIndex - 1];
186+
var nxtLine = diffLines[lineIndex + 1];
187+
var afterNxtLine = diffLines[lineIndex + 2];
188+
189+
if (utils.startsWith(line, 'diff')) {
190+
startFile();
191+
currentFile.isGitDiff = true;
192+
return;
193+
}
194+
195+
if (!currentFile || // If we do not have a file yet, we should crete one
183196
(
184-
currentFile && // If we already have some file in progress and
197+
!currentFile.isGitDiff && currentFile && // If we already have some file in progress and
185198
(
186-
currentFile.oldName && utils.startsWith(line, '--- ') || // Either we reached a old file identification line
187-
currentFile.newName && utils.startsWith(line, '+++ ') // Or we reached a new file identification line
199+
utils.startsWith(line, oldFileNameHeader) && // If we get to an old file path header line
200+
// And is followed by the new file path header line and the hunk header line
201+
utils.startsWith(nxtLine, newFileNameHeader) && utils.startsWith(afterNxtLine, hunkHeaderPrefix)
188202
)
189203
)
190204
) {
@@ -194,28 +208,43 @@
194208
var values;
195209

196210
/*
197-
* --- Date Timestamp[FractionalSeconds] TimeZone
198-
* --- 2002-02-21 23:30:39.942229878 -0800
211+
* We need to make sure that we have the three lines of the header.
212+
* This avoids cases like the ones described in:
213+
* - https://github.com/rtfpessoa/diff2html/issues/87
199214
*/
200-
if (currentFile && !currentFile.oldName &&
201-
utils.startsWith(line, '--- ') && (values = getSrcFilename(line, config))) {
202-
currentFile.oldName = values;
203-
currentFile.language = getExtension(currentFile.oldName, currentFile.language);
204-
return;
205-
}
215+
if (
216+
(utils.startsWith(line, oldFileNameHeader) &&
217+
utils.startsWith(nxtLine, newFileNameHeader) && utils.startsWith(afterNxtLine, hunkHeaderPrefix)) ||
218+
219+
(utils.startsWith(line, newFileNameHeader) &&
220+
utils.startsWith(prevLine, oldFileNameHeader) && utils.startsWith(nxtLine, hunkHeaderPrefix))
221+
) {
222+
223+
/*
224+
* --- Date Timestamp[FractionalSeconds] TimeZone
225+
* --- 2002-02-21 23:30:39.942229878 -0800
226+
*/
227+
if (currentFile && !currentFile.oldName &&
228+
utils.startsWith(line, '--- ') && (values = getSrcFilename(line, config))) {
229+
currentFile.oldName = values;
230+
currentFile.language = getExtension(currentFile.oldName, currentFile.language);
231+
return;
232+
}
233+
234+
/*
235+
* +++ Date Timestamp[FractionalSeconds] TimeZone
236+
* +++ 2002-02-21 23:30:39.942229878 -0800
237+
*/
238+
if (currentFile && !currentFile.newName &&
239+
utils.startsWith(line, '+++ ') && (values = getDstFilename(line, config))) {
240+
currentFile.newName = values;
241+
currentFile.language = getExtension(currentFile.newName, currentFile.language);
242+
return;
243+
}
206244

207-
/*
208-
* +++ Date Timestamp[FractionalSeconds] TimeZone
209-
* +++ 2002-02-21 23:30:39.942229878 -0800
210-
*/
211-
if (currentFile && !currentFile.newName &&
212-
utils.startsWith(line, '+++ ') && (values = getDstFilename(line, config))) {
213-
currentFile.newName = values;
214-
currentFile.language = getExtension(currentFile.newName, currentFile.language);
215-
return;
216245
}
217246

218-
if (currentFile && utils.startsWith(line, '@')) {
247+
if (currentFile && utils.startsWith(line, hunkHeaderPrefix)) {
219248
startBlock(line);
220249
return;
221250
}
@@ -231,13 +260,6 @@
231260
return;
232261
}
233262

234-
if (
235-
(currentFile && currentFile.blocks.length) ||
236-
(currentBlock && currentBlock.lines.length)
237-
) {
238-
startFile();
239-
}
240-
241263
/*
242264
* Git diffs provide more information regarding files modes, renames, copies,
243265
* commits between changes and similarity indexes

‎src/utils.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@
3434
return result;
3535
}
3636

37-
return str.indexOf(start) === 0;
37+
return str && str.indexOf(start) === 0;
3838
};
3939

4040
Utils.prototype.valueOrEmpty = function(value) {

‎test/diff-parser-tests.js

Lines changed: 79 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -424,33 +424,103 @@ describe('DiffParser', function() {
424424
assert.deepEqual(linesContent, ['-test', '+test1r', '+test2r']);
425425
});
426426

427+
it('should parse unified diff with multiple hunks and files', function() {
428+
var diff =
429+
'--- sample.js\n' +
430+
'+++ sample.js\n' +
431+
'@@ -1 +1,2 @@\n' +
432+
'-test\n' +
433+
'@@ -10 +20,2 @@\n' +
434+
'+test\n' +
435+
'--- sample1.js\n' +
436+
'+++ sample1.js\n' +
437+
'@@ -1 +1,2 @@\n' +
438+
'+test1';
439+
440+
var result = DiffParser.generateDiffJson(diff);
441+
assert.equal(2, result.length);
442+
443+
var file1 = result[0];
444+
assert.equal(1, file1.addedLines);
445+
assert.equal(1, file1.deletedLines);
446+
assert.equal('sample.js', file1.oldName);
447+
assert.equal('sample.js', file1.newName);
448+
assert.equal(2, file1.blocks.length);
449+
450+
var linesContent1 = file1.blocks[0].lines.map(function(line) {
451+
return line.content;
452+
});
453+
assert.deepEqual(linesContent1, ['-test']);
454+
455+
var linesContent2 = file1.blocks[1].lines.map(function(line) {
456+
return line.content;
457+
});
458+
assert.deepEqual(linesContent2, ['+test']);
459+
460+
var file2 = result[1];
461+
assert.equal(1, file2.addedLines);
462+
assert.equal(0, file2.deletedLines);
463+
assert.equal('sample1.js', file2.oldName);
464+
assert.equal('sample1.js', file2.newName);
465+
assert.equal(1, file2.blocks.length);
466+
467+
var linesContent = file2.blocks[0].lines.map(function(line) {
468+
return line.content;
469+
});
470+
assert.deepEqual(linesContent, ['+test1']);
471+
});
472+
427473
it('should parse diff with --- and +++ in the context lines', function() {
428474
var diff =
429475
'--- sample.js\n' +
430476
'+++ sample.js\n' +
431-
'@@ -1,15 +1,12 @@\n' +
477+
'@@ -1,8 +1,8 @@\n' +
432478
' test\n' +
433479
' \n' +
434-
'----\n' +
435-
'+test\n' +
480+
'-- 1\n' +
481+
'--- 1\n' +
482+
'---- 1\n' +
436483
' \n' +
437-
' test\n' +
438-
'----\n' +
439-
'\\ No newline at end of file';
484+
'++ 2\n' +
485+
'+++ 2\n' +
486+
'++++ 2';
440487

441488
var result = DiffParser.generateDiffJson(diff);
442489
var file1 = result[0];
443490
assert.equal(1, result.length);
444-
assert.equal(1, file1.addedLines);
445-
assert.equal(2, file1.deletedLines);
491+
assert.equal(3, file1.addedLines);
492+
assert.equal(3, file1.deletedLines);
493+
assert.equal('sample.js', file1.oldName);
494+
assert.equal('sample.js', file1.newName);
495+
assert.equal(1, file1.blocks.length);
496+
497+
var linesContent = file1.blocks[0].lines.map(function(line) {
498+
return line.content;
499+
});
500+
assert.deepEqual(linesContent,
501+
[' test', ' ', '-- 1', '--- 1', '---- 1', ' ', '++ 2', '+++ 2', '++++ 2']);
502+
});
503+
504+
it('should parse diff without proper hunk headers', function() {
505+
var diff =
506+
'--- sample.js\n' +
507+
'+++ sample.js\n' +
508+
'@@ @@\n' +
509+
' test';
510+
511+
var result = DiffParser.generateDiffJson(diff);
512+
var file1 = result[0];
513+
assert.equal(1, result.length);
514+
assert.equal(0, file1.addedLines);
515+
assert.equal(0, file1.deletedLines);
446516
assert.equal('sample.js', file1.oldName);
447517
assert.equal('sample.js', file1.newName);
448518
assert.equal(1, file1.blocks.length);
449519

450520
var linesContent = file1.blocks[0].lines.map(function(line) {
451521
return line.content;
452522
});
453-
assert.deepEqual(linesContent, [' test', ' ', '----', '+test', ' ', ' test', '----']);
523+
assert.deepEqual(linesContent, [' test']);
454524
});
455525

456526
});

0 commit comments

Comments
 (0)
0