TheCBProject / DiffPatch

MIT License
5 stars 15 forks source link

PatchOperation does not handle multiple file removals/deletions #4

Closed AterAnimAvis closed 1 year ago

AterAnimAvis commented 3 years ago

Test Case Relevant Stack Trace

Description: https://github.com/TheCBProject/DiffPatch/blob/8affab08a7e9a98ab52e675adc357af8275e9017/src/main/java/codechicken/diffpatch/cli/PatchOperation.java#L261-L268 Current logic for a removal patch falls through to e.patchedPath which will always be /dev/null. Collectors#toMap will then throw an exception due to the duplicated file keys.

One potential solution: Default to name if the patchedPath is either null or /dev/null Fixing that causes a secondary issue where the target files are still created with empty contents

covers1624 commented 3 years ago

Interesting, this was never considered when I was writing the cli interface.

I suppose it fails similarly when creating files with patches?

AterAnimAvis commented 3 years ago

Looks like it doesn't actually work at all: Missing patch target for A.txt.patch

Test Case

Chicken-Bones commented 3 years ago

I didn't know removal patches were a thing, but this excites me. We should support them when diffing too :)

AterAnimAvis commented 3 years ago

It already does in terms of cli, these patches where generated by DiffOperation.