Preserve "garbage"

README.md

@@ -139,9 +139,13 @@ Broadly, jsdiff's diff functions all take an old text and a new text and perform
 
     Once all patches have been applied or an error occurs, the `options.complete(err)` callback is made.
 
-* `Diff.parsePatch(diffStr)` - Parses a patch into structured data
+* `Diff.parsePatch(diffStr[, options])` - Parses a unified diff format patch (possibly representing multiple diffs, to different files) into structured data.
 
-    Return a JSON object representation of the a patch, suitable for use with the `applyPatch` method. This parses to the same structure returned by `Diff.structuredPatch`.
+    Return an array of objects, one per file diff, suitable for use with the `applyPatch` method. This parses to the same structure returned by `Diff.structuredPatch`, except that each object by default also has a `leadingGarbage` key containing any content preceding the `---` and `+++` file headers.
+
+    The optional `options` argument may have the following keys:
+
+    - `discardGarbage`: if true, `parsePatch` won't set the `leadingGarbage` property.
 
 * `Diff.reversePatch(patch)` - Returns a new structured patch which when applied will undo the original `patch`.
 

release-notes.md

@@ -26,6 +26,7 @@
 - [#489](github.com/kpdecker/jsdiff/pull/489) **`this.options` no longer exists on `Diff` objects.** Instead, `options` is now passed as an argument to methods that rely on options, like `equals(left, right, options)`. This fixes a race condition in async mode, where diffing behaviour could be changed mid-execution if a concurrent usage of the same `Diff` instances overwrote its `options`.
 - [#518](https://github.com/kpdecker/jsdiff/pull/518) **`linedelimiters` no longer exists** on patch objects; instead, when a patch with Windows-style CRLF line endings is parsed, **the lines in `lines` will end with `\r`**. There is now a **new `autoConvertLineEndings` option, on by default**, which makes it so that when a patch with Windows-style line endings is applied to a source file with Unix style line endings, the patch gets autoconverted to use Unix-style line endings, and when a patch with Unix-style line endings is applied to a source file with Windows-style line endings, it gets autoconverted to use Windows-style line endings.
 - [#521](https://github.com/kpdecker/jsdiff/pull/521) **the `callback` option is now supported by `structuredPatch`, `createPatch`, and `createTwoFilesPatch`**
+- [#522](https://github.com/kpdecker/jsdiff/pull/522) **`parsePatch` can now parse patches where lines starting with `--` or `++` are deleted/inserted**; previously, there were edge cases where the parser would choke on valid patches or give wrong results. Also, by default **`parsePatch` now preserves "leading garbage" from before each file in the patch in a `leadingGarbage` key**, and takes an **optional `discardGarbage` option** to disable this.
 
 ## v5.2.0
 

src/patch/create.js

@@ -117,10 +117,19 @@ export function structuredPatch(oldFileName, newFileName, oldStr, newStr, oldHea
       }
     }
 
+    let leadingGarbage;
+    if (oldFileName == newFileName) {
+      leadingGarbage = `Index: ${oldFileName}\n===================================================================`;
+    } else {
+      leadingGarbage = '===================================================================';
+    }
+
+
     return {
-      oldFileName: oldFileName, newFileName: newFileName,
-      oldHeader: oldHeader, newHeader: newHeader,
-      hunks: hunks
+      oldFileName, newFileName,
+      oldHeader, newHeader,
+      hunks,
+      leadingGarbage
     };
   }
 }
@@ -131,10 +140,9 @@ export function formatPatch(diff) {
   }
 
   const ret = [];
-  if (diff.oldFileName == diff.newFileName) {
-    ret.push('Index: ' + diff.oldFileName);
+  if (diff.leadingGarbage) {
+    ret.push(diff.leadingGarbage);
   }
-  ret.push('===================================================================');
   ret.push('--- ' + diff.oldFileName + (typeof diff.oldHeader === 'undefined' ? '' : '\t' + diff.oldHeader));
   ret.push('+++ ' + diff.newFileName + (typeof diff.newHeader === 'undefined' ? '' : '\t' + diff.newHeader));
 

src/patch/parse.js

@@ -1,4 +1,4 @@
-export function parsePatch(uniDiff) {
+export function parsePatch(uniDiff, options = {discardGarbage: false}) {
   let diffstr = uniDiff.split(/\n/),
       list = [],
       i = 0;
@@ -7,12 +7,17 @@ export function parsePatch(uniDiff) {
     let index = {};
     list.push(index);
 
+    const leadingGarbageLines = [];
+
     // Parse diff metadata
     while (i < diffstr.length) {
       let line = diffstr[i];
 
       // File header found, end parsing diff metadata
       if ((/^(\-\-\-|\+\+\+|@@)\s/).test(line)) {
+        if (!options.discardGarbage) {
+          index.leadingGarbage = leadingGarbageLines.join('\n');
+        }
         break;
       }
 
@@ -22,6 +27,7 @@ export function parsePatch(uniDiff) {
         index.index = header[1];
       }
 
+      leadingGarbageLines.push(line);
       i++;
     }
 
@@ -92,17 +98,12 @@ export function parsePatch(uniDiff) {
 
     let addCount = 0,
         removeCount = 0;
-    for (; i < diffstr.length; i++) {
-      // Lines starting with '---' could be mistaken for the "remove line" operation
-      // But they could be the header for the next file. Therefore prune such cases out.
-      if (diffstr[i].indexOf('--- ') === 0
-            && (i + 2 < diffstr.length)
-            && diffstr[i + 1].indexOf('+++ ') === 0
-            && diffstr[i + 2].indexOf('@@') === 0) {
-          break;
-      }
+    for (
+      ;
+      i < diffstr.length && (removeCount < hunk.oldLines || addCount < hunk.newLines || diffstr[i]?.startsWith('\\'));
+      i++
+    ) {
       let operation = (diffstr[i].length == 0 && i != (diffstr.length - 1)) ? ' ' : diffstr[i][0];
-
       if (operation === '+' || operation === '-' || operation === ' ' || operation === '\\') {
         hunk.lines.push(diffstr[i]);
 
@@ -115,7 +116,7 @@ export function parsePatch(uniDiff) {
           removeCount++;
         }
       } else {
-        break;
+        throw new Error(`Hunk at line ${chunkHeaderIndex + 1} contained invalid line ${diffstr[i]}`);
       }
     }
 

test/patch/create.js

@@ -819,6 +819,7 @@ describe('patch/create', function() {
       expect(res).to.eql({
         oldFileName: 'oldfile', newFileName: 'newfile',
         oldHeader: 'header1', newHeader: 'header2',
+        leadingGarbage: '===================================================================',
         hunks: [{
           oldStart: 1, oldLines: 3, newStart: 1, newLines: 3,
           lines: [' line2', ' line3', '-line4', '+line5', '\\ No newline at end of file']
@@ -835,6 +836,7 @@ describe('patch/create', function() {
           expect(res).to.eql({
             oldFileName: 'oldfile', newFileName: 'newfile',
             oldHeader: 'header1', newHeader: 'header2',
+            leadingGarbage: '===================================================================',
             hunks: [{
               oldStart: 1, oldLines: 3, newStart: 1, newLines: 3,
               lines: [' foo', '-bar', '+barcelona', ' baz']
@@ -854,6 +856,7 @@ describe('patch/create', function() {
           expect(res).to.eql({
             oldFileName: 'oldfile', newFileName: 'newfile',
             oldHeader: 'header1', newHeader: 'header2',
+            leadingGarbage: '===================================================================',
             hunks: [{
               oldStart: 1, oldLines: 3, newStart: 1, newLines: 3,
               lines: [' foo', '-bar', '+barcelona', ' baz']
@@ -931,14 +934,13 @@ describe('patch/create', function() {
         }
       ];
       expect(formatPatch(patch)).to.equal(
-        '===================================================================\n' +
+        // TODO: Verify this is a legit patch before merging this PR. What does diff -u emit?
         '--- foo\t2023-12-29 15:48:17.976616966 +0000\n' +
         '+++ bar\t2023-12-29 15:48:21.400516845 +0000\n' +
         '@@ -1,1 +1,1 @@\n' +
         '-xxx\n' +
         '+yyy\n' +
         '\n' +
-        '===================================================================\n' +
         '--- baz\t2023-12-29 15:48:29.376283616 +0000\n' +
         '+++ qux\t2023-12-29 15:48:32.908180343 +0000\n' +
         '@@ -1,1 +1,1 @@\n' +
@@ -948,10 +950,8 @@ describe('patch/create', function() {
     });
     it('should roughly be the inverse of parsePatch', function() {
       // There are so many differences in how a semantically-equivalent patch
-      // can be formatted in unified diff format, AND in JsDiff's structured
-      // patch format as long as https://github.com/kpdecker/jsdiff/issues/434
-      // goes unresolved, that a stronger claim than "roughly the inverse" is
-      // sadly not possible here.
+      // can be formatted in unified diff format that a stronger claim than "roughly the inverse"
+      // is sadly not possible here.
 
       // Check 1: starting with a patch in uniform diff format, does
       // formatPatch(parsePatch(...)) round-trip?