Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions release-notes.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
24 changes: 21 additions & 3 deletions src/patch/line-endings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
)
}))
};
}
Expand All @@ -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('\\')
)
)
);
Expand All @@ -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(
Expand Down
25 changes: 25 additions & 0 deletions test/patch/apply.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down Expand Up @@ -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 =
Expand Down
59 changes: 58 additions & 1 deletion test/patch/line-endings.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand All @@ -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', () => {
Expand Down Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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);
});
});