Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Future 6.0.0 release #446

Merged
merged 15 commits into from
Feb 13, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Next Next commit
Stop treating stuff like vertical tabs as line breaks when dealing wi…
…th unified diffs (#435)

* Add test of vertical tab handling

* Stop treating stuff like vertical tabs as line breaks when dealing with unified diffs

* Add release notes

* Add a consistency test
  • Loading branch information
ExplodingCabbage committed Dec 27, 2023
commit a4eac49d7f3dd93c4a8961765b0c8e3e689fa5f8
6 changes: 6 additions & 0 deletions release-notes.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# Release Notes

## Future breaking 6.0.0 release

[Commits](https://github.com/kpdecker/jsdiff/compare/master...v6.0.0-staging)

- [#435](https://github.com/kpdecker/jsdiff/pull/435) Fix `parsePatch` handling of control characters. `parsePatch` used to interpret various unusual control characters - namely vertical tabs, form feeds, lone carriage returns without a line feed, and EBCDIC NELs - as line breaks when parsing a patch file. This was inconsistent with the behavior of both JsDiff's own `diffLines` method and also the Unix `diff` and `patch` utils, which all simply treat those control characters as ordinary characters. The result of this discrepancy was that some well-formed patches - produced either by `diff` or by JsDiff itself and handled properly by the `patch` util - would be wrongly parsed by `parsePatch`, with the effect that it would disregard the remainder of a hunk after encountering one of these control characters.

## Development

[Commits](https://github.com/kpdecker/jsdiff/compare/v5.1.0...master)
Expand Down
4 changes: 2 additions & 2 deletions src/patch/apply.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ export function applyPatch(source, uniDiff, options = {}) {
}

// Apply the diff to the input
let lines = source.split(/\r\n|[\n\v\f\r\x85]/),
delimiters = source.match(/\r\n|[\n\v\f\r\x85]/g) || [],
let lines = source.split(/\r?\n/),
delimiters = source.match(/\r?\n/g) || [],
hunks = uniDiff.hunks,

compareLine = options.compareLine || ((lineNumber, line, operation, patchContent) => line === patchContent),
Expand Down
4 changes: 2 additions & 2 deletions src/patch/parse.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
export function parsePatch(uniDiff, options = {}) {
let diffstr = uniDiff.split(/\r\n|[\n\v\f\r\x85]/),
delimiters = uniDiff.match(/\r\n|[\n\v\f\r\x85]/g) || [],
let diffstr = uniDiff.split(/\r?\n/),
delimiters = uniDiff.match(/\r?\n/g) || [],
list = [],
i = 0;

Expand Down
87 changes: 87 additions & 0 deletions test/patch/parse.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import {parsePatch} from '../../lib/patch/parse';
import {createPatch} from '../../lib/patch/create';

import {expect} from 'chai';

Expand Down Expand Up @@ -411,5 +412,91 @@ Index: test2
it('should handle OOM case', function() {
parsePatch('Index: \n===================================================================\n--- \n+++ \n@@ -1,1 +1,2 @@\n-1\n\\ No newline at end of file\n+1\n+2\n');
});

it('should treat vertical tabs like ordinary characters', function() {
// Patch below was generated as follows:
// 1. From Node, run:
// fs.writeFileSync("foo", "foo\nbar\vbar\nbaz\nqux")
// fs.writeFileSync("bar", "foo\nbarry\vbarry\nbaz\nqux")
// 2. From shell, run:
// diff -u foo bar > diff.txt
// 3. From Node, run
// fs.readFileSync("diff.txt")
// and copy the string literal you get.
// This patch illustrates how the Unix `diff` and `patch` tools handle
// characters like vertical tabs - namely, they simply treat them as
// ordinary characters, NOT as line breaks. JsDiff used to treat them as
// line breaks but this breaks its parsing of patches like this one; this
// test was added to demonstrate the brokenness and prevent us from
// reintroducing it.
const patch = '--- foo\t2023-12-20 16:11:20.908225554 +0000\n' +
'+++ bar\t2023-12-20 16:11:34.391473579 +0000\n' +
'@@ -1,4 +1,4 @@\n' +
' foo\n' +
'-bar\x0Bbar\n' +
'+barry\x0Bbarry\n' +
' baz\n' +
' qux\n' +
'\\ No newline at end of file\n';

expect(parsePatch(patch)).to.eql([
{
oldFileName: 'foo',
oldHeader: '2023-12-20 16:11:20.908225554 +0000',
newFileName: 'bar',
newHeader: '2023-12-20 16:11:34.391473579 +0000',
hunks: [
{
oldStart: 1,
oldLines: 4,
newStart: 1,
newLines: 4,
lines: [
' foo',
'-bar\x0Bbar',
'+barry\x0Bbarry',
' baz',
' qux',
'\\ No newline at end of file'
],
linedelimiters: [ '\n', '\n', '\n', '\n', '\n', '\n' ]
}
]
}
]);
});

it('should treat vertical tabs in a way consistent with createPatch', function() {
// This is basically the same as the test above, but this time we create
// the patch USING JsDiff instead of testing one created with Unix diff
const patch = createPatch('foo', 'foo\nbar\vbar\nbaz\nqux', 'foo\nbarry\vbarry\nbaz\nqux');

expect(parsePatch(patch)).to.eql([
{
oldFileName: 'foo',
oldHeader: '',
newFileName: 'foo',
newHeader: '',
index: 'foo',
hunks: [
{
oldStart: 1,
oldLines: 4,
newStart: 1,
newLines: 4,
lines: [
' foo',
'-bar\x0Bbar',
'+barry\x0Bbarry',
' baz',
' qux',
'\\ No newline at end of file'
],
linedelimiters: [ '\n', '\n', '\n', '\n', '\n', '\n' ]
}
]
}
]);
});
});
});