diff --git a/release-notes.md b/release-notes.md index c3d1df85..774103ba 100644 --- a/release-notes.md +++ b/release-notes.md @@ -4,6 +4,7 @@ - [#697](https://github.com/kpdecker/jsdiff/pull/697) *`diffJson` now correctly handles JSON objects containing a key named `__proto__`*. (Previously, the returned diff would be as if the `__proto__` key did not exist on either of the objects being diffed.) - [#700](https://github.com/kpdecker/jsdiff/pull/700) *`diffJson` now correctly handles JSON objects containing a non-callable property named `toJSON`* - i.e. it gives such a property no special behaviour whatsoever, just as `JSON.stringify` doesn't. Previously, such properties caused an error to be thrown. (*Callable* `toJSON` properties continue to get the same special behaviour that `JSON.stringify` gives them.) +- [#701](https://github.com/kpdecker/jsdiff/pull/701) *`applyPatch` with `autoConvertLineEndings` on will no longer consider a stray `\r` character occurring at the end of a file without a terminating `\n` character to be a Windows line ending*, and so will no longer strip it when converting from Windows to Unix line endings nor fail to apply a Unix-style patch to a Windows file when the patch introduces such a stray `\r`. ## 9.0.0 diff --git a/src/patch/line-endings.ts b/src/patch/line-endings.ts index d36c7a59..18de247e 100644 --- a/src/patch/line-endings.ts +++ b/src/patch/line-endings.ts @@ -41,7 +41,16 @@ export function winToUnix(patch: StructuredPatch | StructuredPatch[]): Structure ...patch, hunks: patch.hunks.map(hunk => ({ ...hunk, - lines: hunk.lines.map(line => line.endsWith('\r') ? line.substring(0, line.length - 1) : line) + lines: hunk.lines.map( + (line, i) => + // A trailing '\r' on a line immediately followed by a "\ No newline at end of file" + // marker is not a Windows line ending (a Windows EOL is '\r\n'); it's a literal carriage + // return in the final line's content. Stripping it would corrupt the content, so we leave + // such lines alone. (This mirrors the equivalent guard in unixToWin.) + (line.endsWith('\r') && !hunk.lines[i + 1]?.startsWith('\\')) + ? line.substring(0, line.length - 1) + : line + ) })) }; } @@ -55,7 +64,7 @@ export function isUnix(patch: StructuredPatch | StructuredPatch[]): boolean { return !patch.some( index => index.hunks.some( hunk => hunk.lines.some( - line => !line.startsWith('\\') && line.endsWith('\r') + (line, i) => !line.startsWith('\\') && line.endsWith('\r') && !hunk.lines[i + 1]?.startsWith('\\') ) ) ); @@ -66,7 +75,16 @@ export function isUnix(patch: StructuredPatch | StructuredPatch[]): boolean { */ export function isWin(patch: StructuredPatch | StructuredPatch[]): boolean { if (!Array.isArray(patch)) { patch = [patch]; } - return patch.some(index => index.hunks.some(hunk => hunk.lines.some(line => line.endsWith('\r')))) + return patch.some( + index => index.hunks.some( + hunk => hunk.lines.some( + // A trailing '\r' before a "\ No newline at end of file" marker is a literal carriage + // return in the final line's content, not a Windows line ending, so it isn't evidence that + // the patch uses Windows line endings. + (line, i) => line.endsWith('\r') && !hunk.lines[i + 1]?.startsWith('\\') + ) + ) + ) && patch.every( index => index.hunks.every( hunk => hunk.lines.every( diff --git a/test/patch/apply.js b/test/patch/apply.js index c789488e..6cc1ec09 100755 --- a/test/patch/apply.js +++ b/test/patch/apply.js @@ -1422,6 +1422,20 @@ describe('patch/apply', function() { .to.equal(''); }); + it('should correctly apply a Unix patch whose final added line ends with a literal \\r (no newline at EOF) to a Windows file', () => { + // Regression test for bug fixed in https://github.com/kpdecker/jsdiff/pull/701 + // The patch is Unix-style (no \\r\\n line endings), but the added line's content ends with a + // literal '\\r' because the new file has no trailing newline. autoConvertLineEndings must + // recognise the patch as Unix (not Windows), convert it to match the CRLF source, and apply + // it correctly — without dropping the literal '\\r'. Previously, isUnix() returned false for + // such a patch, so no conversion was attempted and applyPatch returned false. + const oldFileUnix = 'line1\nline2\n'; + const newFileUnix = 'line1\nline3\r'; // final line has literal CR and no trailing newline + const patch = structuredPatch('test', 'test', oldFileUnix, newFileUnix, undefined, undefined, {context: 0}); + const oldFileWin = 'line1\r\nline2\r\n'; + expect(applyPatch(oldFileWin, patch)).to.equal('line1\r\nline3\r'); + }); + it('should automatically convert a patch with Unix file endings to Windows when patching a Windows file', () => { const oldFile = 'foo\r\nbar\r\nbaz\r\nqux\r\n'; const diffFile = @@ -1475,6 +1489,17 @@ describe('patch/apply', function() { expect(applyPatch(oldFile2, diffFile)).to.equal('foo\nnew\r\ntwo\r\nthree\r\nqux\n'); }); + it('should not strip a literal carriage return from a no-newline-at-EOF line when patching a Unix file', () => { + // Regression test for bug fixed in https://github.com/kpdecker/jsdiff/pull/701 + // The final line's content ends with a literal '\r' and has no trailing newline. This '\r' is + // not a Windows line ending (those are '\r\n'), so autoConvertLineEndings must not strip it - + // doing so previously corrupted the output, losing the carriage return. + const oldFile = 'line1\nline2\n'; + const newFile = 'line1\nline2\nline3\r'; + const patch = structuredPatch('test', 'test', oldFile, newFile, undefined, undefined, {context: 0}); + expect(applyPatch(oldFile, patch)).to.equal(newFile); + }); + it('should leave patch file endings alone if autoConvertLineEndings=false', () => { const oldFile = 'foo\r\nbar\r\nbaz\r\nqux\r\n'; const diffFile = diff --git a/test/patch/line-endings.js b/test/patch/line-endings.js index 19d318b8..4e95ae78 100644 --- a/test/patch/line-endings.js +++ b/test/patch/line-endings.js @@ -47,7 +47,7 @@ describe('unixToWin and winToUnix', function() { expect(formatPatch(winToUnix(patch))).to.equal(formatPatch(unixPatch)); }); - it('should not introduce \\r on the last line if there was no newline at EOF', () => { + it('unixToWin should not introduce \\r on the last line if there was no newline at EOF', () => { const patch = parsePatch( 'Index: test\n' + '===================================================================\n' @@ -73,6 +73,35 @@ describe('unixToWin and winToUnix', function() { + '\\ No newline at end of file\n' ); }); + + it('winToUnix should not strip a literal \\r from the last line if there was no newline at EOF', () => { + // The final line's content ends with a literal '\r' with no trailing newline. That '\r' is not + // a Windows line ending (those are '\r\n'), so it must be left alone rather than stripped. + const patch = parsePatch( + 'Index: test\n' + + '===================================================================\n' + + '--- test\theader1\n' + + '+++ test\theader2\n' + + '@@ -1,2 +1,2 @@\n' + + ' line2\r\n' + + '-line3\r\n' + + '+line3changed\r\n' + + '\\ No newline at end of file\n' + ); + + const unixPatch = winToUnix(patch); + expect(formatPatch(unixPatch)).to.equal( + 'Index: test\n' + + '===================================================================\n' + + '--- test\theader1\n' + + '+++ test\theader2\n' + + '@@ -1,2 +1,2 @@\n' + + ' line2\n' + + '-line3\n' + + '+line3changed\r\n' + + '\\ No newline at end of file\n' + ); + }); }); describe('isWin', () => { @@ -118,6 +147,19 @@ describe('isWin', () => { ); expect(isWin(patch)).to.equal(true); }); + + it('should return false if the only line to end with a CR is a no-newline-at-EOF line (making it a stray literal CR, not a Windows line ending)', () => { + const patch = parsePatch( + 'Index: test\n' + + '===================================================================\n' + + '--- test\theader1\n' + + '+++ test\theader2\n' + + '@@ -2,0 +3,1 @@\n' + + '+line3\r\n' + + '\\ No newline at end of file\n' + ); + expect(isWin(patch)).to.equal(false); + }); }); describe('isUnix', () => { @@ -148,4 +190,19 @@ describe('isUnix', () => { ); expect(isUnix(patch)).to.equal(true); }); + + it('should still return true if only the last line in a file has a CR and there is a no newline at EOF indicator', () => { + const patch = parsePatch( + 'Index: test\n' + + '===================================================================\n' + + '--- test\theader1\n' + + '+++ test\theader2\n' + + '@@ -1,2 +1,3 @@\n' + + ' line2\n' + + ' line3\n' + + '+line4\r\n' + + '\\ No newline at end of file\n' + ); + expect(isUnix(patch)).to.equal(true); + }); });