From 791ced29da8caf6b6e53d739129860239e02abe0 Mon Sep 17 00:00:00 2001 From: Mark Amery Date: Fri, 7 Jun 2024 11:58:19 +0100 Subject: [PATCH 1/9] Support callback option in structuredPatch --- src/patch/create.js | 178 ++++++++++++++++++++++++-------------------- 1 file changed, 98 insertions(+), 80 deletions(-) diff --git a/src/patch/create.js b/src/patch/create.js index 96a5cc27..e46b4d18 100644 --- a/src/patch/create.js +++ b/src/patch/create.js @@ -8,100 +8,118 @@ export function structuredPatch(oldFileName, newFileName, oldStr, newStr, oldHea options.context = 4; } - const diff = diffLines(oldStr, newStr, options); - if(!diff) { - return; + if (!options.callback) { + return diffLinesResultToPatch(diffLines(oldStr, newStr, options)); + } else { + const {callback} = options; + diffLines( + oldStr, + newStr, + { + ...options, + callback: (diff) => { + const patch = diffLinesResultToPatch(diff); + callback(patch); + } + } + ); } - diff.push({value: '', lines: []}); // Append an empty value to make cleanup easier + function diffLinesResultToPatch(diff) { + if(!diff) { + return; + } - function contextLines(lines) { - return lines.map(function(entry) { return ' ' + entry; }); - } + diff.push({value: '', lines: []}); // Append an empty value to make cleanup easier - let hunks = []; - let oldRangeStart = 0, newRangeStart = 0, curRange = [], - oldLine = 1, newLine = 1; - for (let i = 0; i < diff.length; i++) { - const current = diff[i], - lines = current.lines || current.value.replace(/\n$/, '').split('\n'); - current.lines = lines; - - if (current.added || current.removed) { - // If we have previous context, start with that - if (!oldRangeStart) { - const prev = diff[i - 1]; - oldRangeStart = oldLine; - newRangeStart = newLine; - - if (prev) { - curRange = options.context > 0 ? contextLines(prev.lines.slice(-options.context)) : []; - oldRangeStart -= curRange.length; - newRangeStart -= curRange.length; + function contextLines(lines) { + return lines.map(function(entry) { return ' ' + entry; }); + } + + let hunks = []; + let oldRangeStart = 0, newRangeStart = 0, curRange = [], + oldLine = 1, newLine = 1; + for (let i = 0; i < diff.length; i++) { + const current = diff[i], + lines = current.lines || current.value.replace(/\n$/, '').split('\n'); + current.lines = lines; + + if (current.added || current.removed) { + // If we have previous context, start with that + if (!oldRangeStart) { + const prev = diff[i - 1]; + oldRangeStart = oldLine; + newRangeStart = newLine; + + if (prev) { + curRange = options.context > 0 ? contextLines(prev.lines.slice(-options.context)) : []; + oldRangeStart -= curRange.length; + newRangeStart -= curRange.length; + } } - } - // Output our changes - curRange.push(... lines.map(function(entry) { - return (current.added ? '+' : '-') + entry; - })); + // Output our changes + curRange.push(... lines.map(function(entry) { + return (current.added ? '+' : '-') + entry; + })); - // Track the updated file position - if (current.added) { - newLine += lines.length; - } else { - oldLine += lines.length; - } - } else { - // Identical context lines. Track line changes - if (oldRangeStart) { - // Close out any changes that have been output (or join overlapping) - if (lines.length <= options.context * 2 && i < diff.length - 2) { - // Overlapping - curRange.push(... contextLines(lines)); + // Track the updated file position + if (current.added) { + newLine += lines.length; } else { - // end the range and output - let contextSize = Math.min(lines.length, options.context); - curRange.push(... contextLines(lines.slice(0, contextSize))); - - let hunk = { - oldStart: oldRangeStart, - oldLines: (oldLine - oldRangeStart + contextSize), - newStart: newRangeStart, - newLines: (newLine - newRangeStart + contextSize), - lines: curRange - }; - if (i >= diff.length - 2 && lines.length <= options.context) { - // EOF is inside this hunk - let oldEOFNewline = ((/\n$/).test(oldStr)); - let newEOFNewline = ((/\n$/).test(newStr)); - let noNlBeforeAdds = lines.length == 0 && curRange.length > hunk.oldLines; - if (!oldEOFNewline && noNlBeforeAdds && oldStr.length > 0) { - // special case: old has no eol and no trailing context; no-nl can end up before adds - // however, if the old file is empty, do not output the no-nl line - curRange.splice(hunk.oldLines, 0, '\\ No newline at end of file'); - } - if ((!oldEOFNewline && !noNlBeforeAdds) || !newEOFNewline) { - curRange.push('\\ No newline at end of file'); + oldLine += lines.length; + } + } else { + // Identical context lines. Track line changes + if (oldRangeStart) { + // Close out any changes that have been output (or join overlapping) + if (lines.length <= options.context * 2 && i < diff.length - 2) { + // Overlapping + curRange.push(... contextLines(lines)); + } else { + // end the range and output + let contextSize = Math.min(lines.length, options.context); + curRange.push(... contextLines(lines.slice(0, contextSize))); + + let hunk = { + oldStart: oldRangeStart, + oldLines: (oldLine - oldRangeStart + contextSize), + newStart: newRangeStart, + newLines: (newLine - newRangeStart + contextSize), + lines: curRange + }; + if (i >= diff.length - 2 && lines.length <= options.context) { + // EOF is inside this hunk + let oldEOFNewline = ((/\n$/).test(oldStr)); + let newEOFNewline = ((/\n$/).test(newStr)); + let noNlBeforeAdds = lines.length == 0 && curRange.length > hunk.oldLines; + if (!oldEOFNewline && noNlBeforeAdds && oldStr.length > 0) { + // special case: old has no eol and no trailing context; no-nl can end up before adds + // however, if the old file is empty, do not output the no-nl line + curRange.splice(hunk.oldLines, 0, '\\ No newline at end of file'); + } + if ((!oldEOFNewline && !noNlBeforeAdds) || !newEOFNewline) { + curRange.push('\\ No newline at end of file'); + } } - } - hunks.push(hunk); + hunks.push(hunk); - oldRangeStart = 0; - newRangeStart = 0; - curRange = []; + oldRangeStart = 0; + newRangeStart = 0; + curRange = []; + } } + oldLine += lines.length; + newLine += lines.length; } - oldLine += lines.length; - newLine += lines.length; } - } - return { - oldFileName: oldFileName, newFileName: newFileName, - oldHeader: oldHeader, newHeader: newHeader, - hunks: hunks - }; + return { + oldFileName: oldFileName, newFileName: newFileName, + oldHeader: oldHeader, newHeader: newHeader, + hunks: hunks + }; + } } export function formatPatch(diff) { From 4f0f6df41b25f06898997499f6ddd25fd4394a81 Mon Sep 17 00:00:00 2001 From: Mark Amery Date: Fri, 7 Jun 2024 20:46:58 +0100 Subject: [PATCH 2/9] Test callback in structuredPatch --- test/patch/create.js | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/test/patch/create.js b/test/patch/create.js index 498d14c2..7e534897 100644 --- a/test/patch/create.js +++ b/test/patch/create.js @@ -766,6 +766,25 @@ describe('patch/create', function() { }); }); + it('takes an optional callback option', function(done) { + structuredPatch( + 'oldfile', 'newfile', + 'foo\nbar\nbaz\n', 'foo\nbarcelona\nbaz\n', + 'header1', 'header2', + {callback: (res) => { + expect(res).to.eql({ + oldFileName: 'oldfile', newFileName: 'newfile', + oldHeader: 'header1', newHeader: 'header2', + hunks: [{ + oldStart: 1, oldLines: 3, newStart: 1, newLines: 3, + lines: [' foo', '-bar', '+barcelona', ' baz'] + }] + }); + done(); + }} + ); + }); + describe('given options.maxEditLength', function() { const options = { maxEditLength: 1 }; From f40a899839967b2e47631b5ad179a351423b9245 Mon Sep 17 00:00:00 2001 From: Mark Amery Date: Fri, 7 Jun 2024 20:52:23 +0100 Subject: [PATCH 3/9] Add test of callback option to createPatch --- test/patch/create.js | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/test/patch/create.js b/test/patch/create.js index 7e534897..076dc179 100644 --- a/test/patch/create.js +++ b/test/patch/create.js @@ -708,6 +708,29 @@ describe('patch/create', function() { expect(diffResult).to.equal(expectedResult); }); }); + + + it('takes an optional callback option', function(done) { + createPatch( + 'test', + 'foo\nbar\nbaz\n', 'foo\nbarcelona\nbaz\n', + 'header1', 'header2', + {callback: (res) => { + expect(res).to.eql( + 'Index: test\n' + + '===================================================================\n' + + '--- test\theader1\n' + + '+++ test\theader2\n' + + '@@ -1,3 +1,3 @@\n' + + ' foo\n' + + '-bar\n' + + '+barcelona\n' + + ' baz\n' + ); + done(); + }} + ); + }); }); describe('stripTrailingCr', function() { From 7136afe9a280f6d77bf0fa69f70df8a7ded12e7c Mon Sep 17 00:00:00 2001 From: Mark Amery Date: Fri, 7 Jun 2024 20:56:32 +0100 Subject: [PATCH 4/9] Add callback support to createPatch --- src/patch/create.js | 30 ++++++++++++++++++++++++++---- 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/src/patch/create.js b/src/patch/create.js index e46b4d18..2a020ade 100644 --- a/src/patch/create.js +++ b/src/patch/create.js @@ -158,11 +158,33 @@ export function formatPatch(diff) { } export function createTwoFilesPatch(oldFileName, newFileName, oldStr, newStr, oldHeader, newHeader, options) { - const patchObj = structuredPatch(oldFileName, newFileName, oldStr, newStr, oldHeader, newHeader, options); - if (!patchObj) { - return; + if (!options?.callback) { + const patchObj = structuredPatch(oldFileName, newFileName, oldStr, newStr, oldHeader, newHeader, options); + if (!patchObj) { + return; + } + return formatPatch(patchObj); + } else { + const {callback} = options; + structuredPatch( + oldFileName, + newFileName, + oldStr, + newStr, + oldHeader, + newHeader, + { + ...options, + callback: patchObj => { + if (!patchObj) { + callback(); + } else { + callback(formatPatch(patchObj)); + } + } + } + ); } - return formatPatch(patchObj); } export function createPatch(fileName, oldStr, newStr, oldHeader, newHeader, options) { From 4e10e80591d4abd05f6b5924bba7e5cf501faba6 Mon Sep 17 00:00:00 2001 From: Mark Amery Date: Fri, 7 Jun 2024 21:46:26 +0100 Subject: [PATCH 5/9] Add one more test to get over the coverage threshold --- test/patch/create.js | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/test/patch/create.js b/test/patch/create.js index 076dc179..02bc75cb 100644 --- a/test/patch/create.js +++ b/test/patch/create.js @@ -731,6 +731,21 @@ describe('patch/create', function() { }} ); }); + + it('still supports early termination when in async mode', function(done) { + createPatch( + 'test', + 'foo\nbar\nbaz\n', 'food\nbarcelona\nbaz\n', + 'header1', 'header2', + { + maxEditLength: 1, + callback: (res) => { + expect(res).to.eql(undefined); + done(); + } + } + ); + }); }); describe('stripTrailingCr', function() { From 9de7f97573a7947e4f4c1d36f1c8e6acd25bc1ca Mon Sep 17 00:00:00 2001 From: Mark Amery Date: Fri, 7 Jun 2024 21:52:56 +0100 Subject: [PATCH 6/9] Update README --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 68e26185..67010ac5 100644 --- a/README.md +++ b/README.md @@ -153,9 +153,9 @@ Broadly, jsdiff's diff functions all take an old text and a new text and perform #### Universal `options` -Certain options can be provided in the `options` object of *any* method that calculates a diff: +Certain options can be provided in the `options` object of *any* method that calculates a diff (including `diffChars`, `diffLines` etc. as well as `structuredPatch`, `createPatch`, and `createTwoFilesPatch`): -* `callback`: if provided, the diff will be computed in async mode to avoid blocking the event loop while the diff is calculated. The value of the `callback` option should be a function and will be passed the result of the diff as its first argument. Only works with functions that return change objects, like `diffLines`, not those that return patches, like `structuredPatch` or `createPatch`. +* `callback`: if provided, the diff will be computed in async mode to avoid blocking the event loop while the diff is calculated. The value of the `callback` option should be a function and will be passed the computed diff or patch as its first argument. (Note that if the ONLY option you want to provide is a callback, you can pass the callback function directly as the `options` parameter instead of passing an object with a `callback` property.) From 146a61f28b8ca6ddbc44bb35ebe39512f3755742 Mon Sep 17 00:00:00 2001 From: Mark Amery Date: Fri, 7 Jun 2024 21:53:16 +0100 Subject: [PATCH 7/9] Support passing function as 'options' argument as with existing methods --- src/patch/create.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/patch/create.js b/src/patch/create.js index 2a020ade..ed3a2718 100644 --- a/src/patch/create.js +++ b/src/patch/create.js @@ -4,6 +4,9 @@ export function structuredPatch(oldFileName, newFileName, oldStr, newStr, oldHea if (!options) { options = {}; } + if (typeof options === 'function') { + options = {callback: options}; + } if (typeof options.context === 'undefined') { options.context = 4; } @@ -158,6 +161,10 @@ export function formatPatch(diff) { } export function createTwoFilesPatch(oldFileName, newFileName, oldStr, newStr, oldHeader, newHeader, options) { + if (typeof options === 'function') { + options = {callback: options}; + } + if (!options?.callback) { const patchObj = structuredPatch(oldFileName, newFileName, oldStr, newStr, oldHeader, newHeader, options); if (!patchObj) { From 707ed3f95d390d0849e386823a9a45f775d103f6 Mon Sep 17 00:00:00 2001 From: Mark Amery Date: Fri, 7 Jun 2024 21:56:20 +0100 Subject: [PATCH 8/9] Add release notes --- release-notes.md | 1 + 1 file changed, 1 insertion(+) diff --git a/release-notes.md b/release-notes.md index d6e4aa3f..b905801e 100644 --- a/release-notes.md +++ b/release-notes.md @@ -25,6 +25,7 @@ - [#490](https://github.com/kpdecker/jsdiff/pull/490) **When calling diffing functions in async mode by passing a `callback` option, the diff result will now be passed as the *first* argument to the callback instead of the second.** (Previously, the first argument was never used at all and would always have value `undefined`.) - [#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`** ## v5.2.0 From 60d4368eeb4b5a9bc9e11ed8cb0211964ae298b7 Mon Sep 17 00:00:00 2001 From: Mark Amery Date: Fri, 7 Jun 2024 21:58:30 +0100 Subject: [PATCH 9/9] Add more tests to meet coverage threshold --- test/patch/create.js | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/test/patch/create.js b/test/patch/create.js index 02bc75cb..7811b3b8 100644 --- a/test/patch/create.js +++ b/test/patch/create.js @@ -732,6 +732,28 @@ describe('patch/create', function() { ); }); + it('lets you provide a callback by passing a function as the `options` parameter', function(done) { + createPatch( + 'test', + 'foo\nbar\nbaz\n', 'foo\nbarcelona\nbaz\n', + 'header1', 'header2', + res => { + expect(res).to.eql( + 'Index: test\n' + + '===================================================================\n' + + '--- test\theader1\n' + + '+++ test\theader2\n' + + '@@ -1,3 +1,3 @@\n' + + ' foo\n' + + '-bar\n' + + '+barcelona\n' + + ' baz\n' + ); + done(); + } + ); + }); + it('still supports early termination when in async mode', function(done) { createPatch( 'test', @@ -823,6 +845,25 @@ describe('patch/create', function() { ); }); + it('lets you provide a callback by passing a function as the `options` parameter', function(done) { + structuredPatch( + 'oldfile', 'newfile', + 'foo\nbar\nbaz\n', 'foo\nbarcelona\nbaz\n', + 'header1', 'header2', + res => { + expect(res).to.eql({ + oldFileName: 'oldfile', newFileName: 'newfile', + oldHeader: 'header1', newHeader: 'header2', + hunks: [{ + oldStart: 1, oldLines: 3, newStart: 1, newLines: 3, + lines: [' foo', '-bar', '+barcelona', ' baz'] + }] + }); + done(); + } + ); + }); + describe('given options.maxEditLength', function() { const options = { maxEditLength: 1 };