Patches created with v5 can not be applied with v7

[markdon]

Repro: https://codesandbox.io/p/devbox/x5s3pc

/** Combine a set of revisions in to their full text */
export async function textOfRevisions(_revisions: Revision[]) {
  const revisions = assertZeroRevision(_revisions);
  return revisions.reduce((acc, r, index) => {
    if (r.sequence === 0) return r.text;
    const thisText = diff7.applyPatch(acc, r.text); // VERSION 7 FAILS TO APPLY
    // const thisText = diff5.applyPatch(acc, r.text);
    if (thisText === false) throw new Error(`Failed to apply patch ${index}`);
    if (!md5Match(thisText, r.checksum)) {
      throw new Error(`Checksum mismatch on revision ${index}`);
    }
    return thisText;
  }, "");
}

I'm storing a full original text document and patches for updates to that document.

After updating diff to version 7, the patches I had stored could not be applied.
diff.applyPatch(<originalText>, <patch>); would return false.

Is this expected as part of a breaking change? I don't see any mention of breaking changes or migration needed.

[ExplodingCabbage]

Hmm, interesting.

Looking at the release notes, the only thing I spot that I'd expect to be pertinent is this:

#535 A bug in patch generation functions is now fixed that would sometimes previously cause \ No newline at end of file to appear in the wrong place in the generated patch, resulting in the patch being invalid.

Perhaps that's relevant here - i.e. perhaps v5 was generating a patch file that wasn't actually a valid patch file (in the sense of being something diff -u would generate or patch would be able to apply), and now v7 is saying "nope, that's not valid" and refusing to apply it? If so, then I think I would view this as a correct change that I wouldn't want to undo - but which I seem to have omitted from the release notes, and maybe not even realised I'd made at the time. I'll fix the release notes, if that's the case.

Alternatively, perhaps your repro involves a valid patch that GNU patch would happily apply, and now we're rejecting it because I royally fucked up applyPatch somehow? If that's the case, then aaaaaaaaaaaaaah.

Time for me to figure out which!

[ExplodingCabbage]

I think it's the thing I speculated above. If I use jsdiff v5 to generate a patch between 0.md and 1.md in your fixture, I get this, which seems nonsensical:

Index: blablabla
===================================================================
--- blablabla
+++ blablabla
@@ -42,22 +42,21 @@
 somewhere in the park. Her eyes were red from crying, and she looked utterly
 heartbroken.
 
 Max, sensing her distress, trotted over and nuzzled her hand with his wet nose.
-Emily's tears slowed as she looked down at the friendly puppy. Mr. Johnson had
-an idea. "Why don't we let Max help us find your teddy bear?" he suggested.
+Emily's tears slowed as she looked down at the friendly puppy. However, Max,
+being Max, wasn't quite done making his mark. Before anyone could react, Max
+lifted his leg and began to wee, and unfortunately, Emily was right in the line
+of fire. She shrieked in surprise, jumping up from the bench.
 
-Emily nodded, and with that, Max was off. His keen sense of smell led him on a
-zigzagging path through the park. He sniffed at bushes, trees, and benches,
-occasionally stopping to leave his mark. After what seemed like hours but was
-only a few minutes, Max's nose caught a familiar scent. He bounded over to a
-large oak tree and began digging at its base.
+Mr. Johnson, mortified, quickly grabbed Max and apologized profusely to Emily
+and her mother, who had rushed over upon hearing her daughter's cry. Emily's
+mother reassured Mr. Johnson that it was alright, even though Emily was still in
+shock from the unexpected shower. They exchanged a few awkward smiles and
+laughed nervously at the situation.
 
\ No newline at end of file
-Buried beneath a pile of leaves was Emily's teddy bear, slightly dirty but
-otherwise intact. Emily's face lit up with joy as she hugged Max tightly. "Thank
-you, Max!" she exclaimed, her sadness replaced with happiness.
-
-From that day on, Max was known not just as the puppy who loved to wee on things
-but also as the hero of the park. The Johnsons continued to love and cherish
-their quirky puppy, who had a heart as big as his bladder. And Max, well, he
-continued to wee on things, but now it was just another endearing part of his
-charm.
+From that day on, Max's reputation in the park was solidified. He was known not
+just as the puppy who loved to wee on things but also as the one who had
+inadvertently marked poor Emily. Despite this, the Johnsons continued to love
+and cherish their quirky puppy, who had a heart as big as his bladder. And Max,
+well, he continued to wee on things, but now with a bit more supervision,
+ensuring he wouldn't repeat his unfortunate mistake.
\ No newline at end of file

The first \ No newline at end of file appears in a nonsensical place. It's neither after the last insertion (in which case it would signify that in the patched file, there is no newline at the end), nor after the last deletion (in which case it would signify the same thing about the unpatched file), nor after a context line at the end of the patch (in which case it would signify that both files end without a newline). Instead it appears somewhere that has no established meaning. And sure enough, if we try to apply the patch with GNU patch, it complains that the patch is invalid:

mark@fractal:~/bugrepro$ patch src/fixtures/max/data/0.md patch5.patch -o -
patching file - (read from src/fixtures/max/data/0.md)
patch: **** malformed patch at line 28: \ No newline at end of file

So I'm happy about v7 rejecting this (it should! The patch is nonsense!) but I should update the release notes to note the change. I'm not sure if I ever actually realised that v5 was able to apply these invalid patches successfully; I think perhaps I accidentally or unthinkingly fixed it in the course of fixing the broken logic that was generating the invalid patches and never consciously registered that it was a distinct change that also needed release notes.

[markdon]

@ExplodingCabbage Thanks for giving this a detailed look!

Personally, my app isn't in production, so I can just wipe my database and import fixtures with v7 and everything is hunky dory.

This seems like it could block moving to v7 for anyone with patches they need to keep. Do you have any advice for anyone that wants to look in to migrating their patches to be valid? Just remove all instances of \ No newline at end of file that occur mid-patch?