.applyPatches() returns false when an operator-less unchanged line mismatches

[saschanaz]

I cooked a repro: https://github.com/saschanaz/jsdiff-test

The patch: https://s3.amazonaws.com/snyk-rules-pre-repository/snapshots/master/patches/npm/qs/20140806-1/qs_20140806-1_0_0_snyk.patch
The target: https://unpkg.com/qs@0.6.6/index.js

The patched callback from .applyPatches() somehow gives false.

GNU patch can apply it without an issue.

[saschanaz]

Found that this is because the default behavior for mismatching unchanged line is different between jsdiff and GNU patch:

• jsdiff always returns false if an unchanged line mismatches
• GNU patch fails only when the line is marked with ! operator.

Should we do the same?

[ExplodingCabbage]

Unfortunately the example patch is no longer publicly available on S3.

GNU patch fails only when the line is marked with ! operator.

This doesn't sound right; !-prefixed lines aren't a thing in unified diff format at all. patch will also fail to apply a hunk if you've changed context lines ("operator-less" lines) that the --fuzz factor doesn't permit to vary. Given the way --fuzz works with patch (it's not quite the maximum number of lines of context that can differ between source file and patch), a single line of context mismatching can cause patch to fail to apply a patch even with the default --fuzz of 2 (e.g. if the mismatching line is immediately before or after the line that the patch changes, and the patch contains 3 or more lines of context on each side of the line that the patch changes):

$ echo 'line1
> line2
> line3
> line4
> line5
> line6
> line7' > foo
$ echo 'line1
> line2
> line3
> lineCHANGED
> line5
> line6
> line7' > bar
$ diff -u foo bar > mydiff.patch
$ cat mydiff.patch 
--- foo 2024-01-03 16:41:55.238883237 +0000
+++ bar 2024-01-03 16:41:55.238883237 +0000
@@ -1,7 +1,7 @@
 line1
 line2
 line3
-line4
+lineCHANGED
 line5
 line6
 line7
$ patch foo mydiff.patch -o -
patching file - (read from foo)
line1
line2
line3
lineCHANGED
line5
line6
line7
$ echo 'line1
> line2
> line3
> line4
> lineX
> line6
> line7' > foo
$ patch foo mydiff.patch -o -
patching file - (read from foo)
Hunk #1 FAILED at 1.
line1
line2
line3
line4
lineX
line6
line7
1 out of 1 hunk FAILED -- saving rejects to file -.rej

So I don't think there's really a bug here? We're behaving along the same lines as patch, just we default to a fuzzFactor of 0 and the precise way our fuzzy matching works is different to patch.

I do think we should document the ways that our fuzzy matching differs from GNU patch, though.

[ExplodingCabbage]

Ugh, it looks like fuzzFactor actually works in a way that is pretty bizarre / broken: #468.

I guess I don't want to add docs for fuzzFactor until I've fixed that, because I'd either have to 1. lie or 2. document behaviour that is crazy and wrong, which people may then come to expect. So I'm going to hold off on fixing this issue (about docs) until I've fixed that issue (about mismatching deletions). Alas, since that's a breaking change, that means no fix until the next breaking release.

[ExplodingCabbage]

I've noted on #468 that docs are needed, so I'm gonna close this issue as now being kinda contained within that one.