From f2051cd5848929658176a04a5e83eae830d5dd49 Mon Sep 17 00:00:00 2001 From: maximilliangrand <214999687+maximilliangrand@users.noreply.github.com> Date: Sun, 16 Aug 2026 11:31:15 +0200 Subject: [PATCH 1/9] fix: don't treat a literal CR at EOF as a Windows line ending `isWin` and `winToUnix` treated any patch line ending in `\r` as a Windows line ending. But a line whose content ends in `\r` and is immediately followed by a `\ No newline at end of file` marker is not a CRLF ending (those are `\r\n`) - it's a literal carriage return in the final line's content. A genuine Windows no-newline-at-EOF line never ends in `\r`. Because of this, `isWin` misclassified such patches as Windows, and when `applyPatch` auto-converted them to match a Unix source, `winToUnix` stripped the `\r`, silently corrupting the output. For example `applyPatch('line1\nline2\n', structuredPatch('f','f','line1\nline2\n', 'line1\nline2\nline3\r', undefined, undefined, {context: 0}))` returned `line1\nline2\nline3` instead of `line1\nline2\nline3\r`. `unixToWin` already guards against this case (it won't add `\r` to a no-newline final line); this makes `isWin` and `winToUnix` symmetric. Co-Authored-By: Claude Opus 4.8 --- src/patch/line-endings.ts | 22 ++++++++++++++++++-- test/patch/apply.js | 10 +++++++++ test/patch/line-endings.js | 42 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 72 insertions(+), 2 deletions(-) diff --git a/src/patch/line-endings.ts b/src/patch/line-endings.ts index d36c7a59..20631101 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 + ) })) }; } @@ -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..a8390266 100755 --- a/test/patch/apply.js +++ b/test/patch/apply.js @@ -1475,6 +1475,16 @@ 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', () => { + // 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..a7d9cf3c 100644 --- a/test/patch/line-endings.js +++ b/test/patch/line-endings.js @@ -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 ending in a CR is a no-newline-at-EOF line (a 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', () => { From 1b880455ef15f6f4e62eb7db4999a0c146f235e8 Mon Sep 17 00:00:00 2001 From: Mark Amery Date: Mon, 17 Aug 2026 13:19:40 +0100 Subject: [PATCH 2/9] Clarify a test description --- test/patch/line-endings.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/patch/line-endings.js b/test/patch/line-endings.js index a7d9cf3c..92e1759c 100644 --- a/test/patch/line-endings.js +++ b/test/patch/line-endings.js @@ -148,7 +148,7 @@ describe('isWin', () => { expect(isWin(patch)).to.equal(true); }); - it('should return false if the only line ending in a CR is a no-newline-at-EOF line (a literal CR, not a Windows line ending)', () => { + 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' From 0c98d460066b3b6506a66e1d57514a649643fcd3 Mon Sep 17 00:00:00 2001 From: Mark Amery Date: Mon, 17 Aug 2026 13:31:13 +0100 Subject: [PATCH 3/9] Add symmetry to names of tests relating to unixToWin & winToUnix --- test/patch/line-endings.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/patch/line-endings.js b/test/patch/line-endings.js index 92e1759c..ca99c3ef 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' From 4a6dfdb6169259bdfcb2c31529920c888541290b Mon Sep 17 00:00:00 2001 From: Mark Amery Date: Mon, 17 Aug 2026 13:34:24 +0100 Subject: [PATCH 4/9] Add release notes --- release-notes.md | 1 + 1 file changed, 1 insertion(+) diff --git a/release-notes.md b/release-notes.md index c3d1df85..f80068e3 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. ## 9.0.0 From 24cc3065d59045a13461780f7f1f2eb06faaaa1f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 17 Aug 2026 14:01:48 +0000 Subject: [PATCH 5/9] Initial plan From 291addd64dd8fa9606667453f43fcdb6d8b8c89a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 17 Aug 2026 14:06:31 +0000 Subject: [PATCH 6/9] fix: exempt lines before no-newline marker from CR check in isUnix Co-authored-by: ExplodingCabbage <2358339+ExplodingCabbage@users.noreply.github.com> --- src/patch/line-endings.ts | 2 +- test/patch/line-endings.js | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/src/patch/line-endings.ts b/src/patch/line-endings.ts index d36c7a59..d6c15b72 100644 --- a/src/patch/line-endings.ts +++ b/src/patch/line-endings.ts @@ -55,7 +55,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('\\') ) ) ); diff --git a/test/patch/line-endings.js b/test/patch/line-endings.js index 19d318b8..3e7728a0 100644 --- a/test/patch/line-endings.js +++ b/test/patch/line-endings.js @@ -148,4 +148,19 @@ describe('isUnix', () => { ); expect(isUnix(patch)).to.equal(true); }); + + it('should still return true if only the last line in a file is missing a LF 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); + }); }); From 0eaa834c1d4cd4ba7352032da79ebec45612f483 Mon Sep 17 00:00:00 2001 From: Mark Amery Date: Mon, 17 Aug 2026 21:45:34 +0100 Subject: [PATCH 7/9] Fix nonsensical test description --- test/patch/line-endings.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/patch/line-endings.js b/test/patch/line-endings.js index 3e7728a0..7ac8650a 100644 --- a/test/patch/line-endings.js +++ b/test/patch/line-endings.js @@ -149,7 +149,7 @@ describe('isUnix', () => { expect(isUnix(patch)).to.equal(true); }); - it('should still return true if only the last line in a file is missing a LF and there is a no newline at EOF indicator', () => { + 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' From 396e86fdb9f81e95742ffc62c45fca45635df2a1 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 17 Aug 2026 20:50:19 +0000 Subject: [PATCH 8/9] test: add applyPatch regression test for Unix patch with literal CR on CRLF source; update release notes Co-authored-by: ExplodingCabbage <2358339+ExplodingCabbage@users.noreply.github.com> --- release-notes.md | 1 + test/patch/apply.js | 13 +++++++++++++ 2 files changed, 14 insertions(+) diff --git a/release-notes.md b/release-notes.md index c3d1df85..33ea391f 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 or 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/test/patch/apply.js b/test/patch/apply.js index c789488e..ec09c1d3 100755 --- a/test/patch/apply.js +++ b/test/patch/apply.js @@ -1422,6 +1422,19 @@ 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', () => { + // 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 = From ddb03b63da8c03a061d8b7b869edbacc27f18c38 Mon Sep 17 00:00:00 2001 From: Mark Amery Date: Tue, 18 Aug 2026 11:03:26 +0100 Subject: [PATCH 9/9] Minor prose tweaks --- release-notes.md | 2 +- test/patch/apply.js | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/release-notes.md b/release-notes.md index 33ea391f..774103ba 100644 --- a/release-notes.md +++ b/release-notes.md @@ -4,7 +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 or fail to apply a Unix-style patch to a Windows file when the patch introduces such a stray `\r`. +- [#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/test/patch/apply.js b/test/patch/apply.js index ca0a44b3..6cc1ec09 100755 --- a/test/patch/apply.js +++ b/test/patch/apply.js @@ -1423,6 +1423,7 @@ describe('patch/apply', function() { }); 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 @@ -1489,6 +1490,7 @@ describe('patch/apply', function() { }); 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.