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

latest version breaks grunt-patcher #238

Closed
leider opened this issue Dec 15, 2018 · 3 comments
Closed

latest version breaks grunt-patcher #238

leider opened this issue Dec 15, 2018 · 3 comments

Comments

@leider
Copy link

leider commented Dec 15, 2018

I am using grunt-patcher and since your latest release (which loaded by npm update on my system) patching fails. Files attached.

demo.zip

@ExplodingCabbage
Copy link
Collaborator

GNU patch 2.7.6 also fails to apply flaticon.patch to flaticon.css on my machine if I use -F 0 to disable fuzzy-matching. I think the problem is that the hunk size in the hunk header is off by 2; it's 24 but should be larger (25 by my counting, though patch wants 26 for some puzzling reason; jsdiff will accept either). I therefore don't think this indicates a bug in jsdiff... although it'd be helpful if we threw an error with a meaningful message saying "Oi, your hunk header says the hunk has 24 lines but the content below the header contains 25 lines; why are you giving me a self-contradicting patch?" rather than just returning false with no explanation! It took me a painfully long time of manually tinkering with the patch and retrying applying it just to figure out why that particular patch wasn't applying.

@ExplodingCabbage
Copy link
Collaborator

parsePatch with {strict: true} seems to have this check:

Uncaught Error: Added line count did not match for hunk at line 8
    at parseHunk (/home/mark/marktest/node_modules/diff/lib/patch/parse.js:150:15)
    at parseIndex (/home/mark/marktest/node_modules/diff/lib/patch/parse.js:54:26)
    at Object.parsePatch (/home/mark/marktest/node_modules/diff/lib/patch/parse.js:162:5)

It'd just be nice if applyPatch did the same thing...

@ExplodingCabbage ExplodingCabbage closed this as not planned Won't fix, can't repro, duplicate, stale Jan 2, 2024
@ExplodingCabbage
Copy link
Collaborator

I suppose it'd also be helpful if, when attempting to apply a structured patch object where the line counts in oldLines and newLines don't match what's indicated by the lines array, we deferred to the content of the array and simply ignored the line count.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants