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

Always enable "strict mode" in parsePatch #508

Merged
merged 32 commits into from
Mar 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
0a3e140
Remove 'strict' option; make strict mode always-on
ExplodingCabbage Mar 20, 2024
f0e78ae
Support multi-file patches with no 'Index: ' or 'diff ' line but with…
ExplodingCabbage Mar 20, 2024
c2cac94
Fix wrong line numbers in a patch in the tests
ExplodingCabbage Mar 20, 2024
33940b3
Fix wrong line numbers in another patch in the tests
ExplodingCabbage Mar 20, 2024
ba3ff94
Fix wrong line numbers in another test
ExplodingCabbage Mar 20, 2024
011e3b8
Fix wrong line numbers in another test
ExplodingCabbage Mar 20, 2024
fd7458f
Fix wrong line numbers in another test
ExplodingCabbage Mar 20, 2024
82da1a2
Fix wrong line numbers in another test
ExplodingCabbage Mar 20, 2024
804caa5
Fix wrong line numbers in another test
ExplodingCabbage Mar 20, 2024
f3ff7cb
Fix wrong line numbers in another test
ExplodingCabbage Mar 20, 2024
8c7f280
Fix wrong line numbers in another test
ExplodingCabbage Mar 20, 2024
51e54eb
Fix wrong line numbers in another test
ExplodingCabbage Mar 20, 2024
fbb4fa4
Fix wrong line numbers in another test
ExplodingCabbage Mar 20, 2024
3a73a48
Fix wrong line numbers in another test
ExplodingCabbage Mar 20, 2024
a6953c2
Fix wrong line numbers in another test
ExplodingCabbage Mar 20, 2024
362aa94
Fix wrong line numbers in another test
ExplodingCabbage Mar 20, 2024
5b3a73c
Fix wrong line numbers in another test
ExplodingCabbage Mar 20, 2024
d6457cb
Fix wrong line numbers in another test
ExplodingCabbage Mar 20, 2024
bade558
Fix wrong line numbers in another test
ExplodingCabbage Mar 20, 2024
f377aa5
Fix wrong line numbers in another test
ExplodingCabbage Mar 20, 2024
e6d96c3
Fix wrong line numbers in another test
ExplodingCabbage Mar 20, 2024
d8ee0a4
Fix wrong line numbers in another test
ExplodingCabbage Mar 20, 2024
a219fed
Fix wrong line numbers in another test (but it still fails - interest…
ExplodingCabbage Mar 20, 2024
6e6a76a
Fix wrong line numbers in another test
ExplodingCabbage Mar 20, 2024
6fd385e
Fix wrong line numbers in another test
ExplodingCabbage Mar 20, 2024
57ba0b1
Fix wrong line numbers in another test
ExplodingCabbage Mar 20, 2024
8ebe646
Fix wrong line numbers in another test
ExplodingCabbage Mar 20, 2024
009a79b
Fix wrong line numbers in another test
ExplodingCabbage Mar 20, 2024
b5c8f14
Fix wrong line numbers in another test
ExplodingCabbage Mar 20, 2024
be766d1
Fix wrong line numbers in another test
ExplodingCabbage Mar 20, 2024
6a9bde9
Remove a failing test whose intention I can't make sense of. What doe…
ExplodingCabbage Mar 20, 2024
dec3099
Add release notes
ExplodingCabbage Mar 20, 2024
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 @@ -13,6 +13,7 @@
**`diffLines` with `ignoreWhitespace: true` will no longer ignore the insertion or deletion of entire extra lines of whitespace at the end of the text**. Previously, these would not show up as insertions or deletions, as a side effect of a hack in the base diffing algorithm meant to help ignore whitespace in `diffWords`. More generally, **the undocumented special handling in the core algorithm for ignored terminals has been removed entirely.** (This special case behavior used to rewrite the final two change objects in a scenario where the final change object was an addition or deletion and its `value` was treated as equal to the empty string when compared using the diff object's `.equals` method.)

- [#500](https://github.com/kpdecker/jsdiff/pull/500) **`diffChars` now diffs Unicode code points** instead of UTF-16 code units.
- [#508](https://github.com/kpdecker/jsdiff/pull/508) **`parsePatch` now always runs in what was previously "strict" mode; the undocumented `strict` option has been removed.** Previously, by default, `parsePatch` (and other patch functions that use it under the hood to parse patches) would accept a patch where the line counts in the headers were inconsistent with the actual patch content - e.g. where a hunk started with the header `@@ -1,3 +1,6 @@`, indicating that the content below spanned 3 lines in the old file and 6 lines in the new file, but then the actual content below the header consisted of some different number of lines, say 10 lines of context, 5 deletions, and 1 insertion. Actually trying to work with these patches using `applyPatch` or `merge`, however, would produce incorrect results instead of just ignoring the incorrect headers, making this "feature" more of a trap than something actually useful. It's been ripped out, and now we are always "strict" and will reject patches where the line counts in the headers aren't consistent with the actual patch content.
- [#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.
- [#439](https://github.com/kpdecker/jsdiff/pull/439) **Prefer diffs that order deletions before insertions.** When faced with a choice between two diffs with an equal total edit distance, the Myers diff algorithm generally prefers one that does deletions before insertions rather than insertions before deletions. For instance, when diffing `abcd` against `acbd`, it will prefer a diff that says to delete the `b` and then insert a new `b` after the `c`, over a diff that says to insert a `c` before the `b` and then delete the existing `c`. JsDiff deviated from the published Myers algorithm in a way that led to it having the opposite preference in many cases, including that example. This is now fixed, meaning diffs output by JsDiff will more accurately reflect what the published Myers diff algorithm would output.
- [#455](https://github.com/kpdecker/jsdiff/pull/455) **The `added` and `removed` properties of change objects are now guaranteed to be set to a boolean value.** (Previously, they would be set to `undefined` or omitted entirely instead of setting them to false.)
Expand Down
22 changes: 9 additions & 13 deletions src/patch/parse.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
export function parsePatch(uniDiff, options = {}) {
export function parsePatch(uniDiff) {
let diffstr = uniDiff.split(/\r?\n/),
delimiters = uniDiff.match(/\r?\n/g) || [],
list = [],
Expand Down Expand Up @@ -36,13 +36,11 @@ export function parsePatch(uniDiff, options = {}) {

while (i < diffstr.length) {
let line = diffstr[i];

if ((/^(Index:|diff|\-\-\-|\+\+\+)\s/).test(line)) {
if ((/^(Index:\s|diff\s|\-\-\-\s|\+\+\+\s|===================================================================)/).test(line)) {
break;
} else if ((/^@@/).test(line)) {
index.hunks.push(parseHunk());
} else if (line && options.strict) {
// Ignore unexpected content unless in strict mode
} else if (line) {
throw new Error('Unknown line ' + (i + 1) + ' ' + JSON.stringify(line));
} else {
i++;
Expand Down Expand Up @@ -132,14 +130,12 @@ export function parsePatch(uniDiff, options = {}) {
hunk.oldLines = 0;
}

// Perform optional sanity checking
if (options.strict) {
if (addCount !== hunk.newLines) {
throw new Error('Added line count did not match for hunk at line ' + (chunkHeaderIndex + 1));
}
if (removeCount !== hunk.oldLines) {
throw new Error('Removed line count did not match for hunk at line ' + (chunkHeaderIndex + 1));
}
// Perform sanity checking
if (addCount !== hunk.newLines) {
throw new Error('Added line count did not match for hunk at line ' + (chunkHeaderIndex + 1));
}
if (removeCount !== hunk.oldLines) {
throw new Error('Removed line count did not match for hunk at line ' + (chunkHeaderIndex + 1));
}

return hunk;
Expand Down
Loading