I've decided this may have been misguided. It's way too complicated and the correct behaviours to implement are not clear. Several dilemmas:
Should we also support parsing/preserving trailing garbage? I can't find any programs that output it, yet the GNU patch util supports ignoring it. What's the right behaviour?
What about trailing garbage after a hunk header, on the same line as it? Git heuristically generates such garbage to try to show the enclosing context (e.g. function declaration) that a hunk appears in - e.g. the export function formatPatch(diff) { here:
if we don't support that, we're still not really letting people roundtrip their patches through a parsePatch / formatPatch combo.
What should formatPatch do if someone passes it a structured patch object where they've manually set the leadingGarbage to include file headers - i.e. to not be valid garbage? We should probably reject that somehow, but that further complicated the API.
Not worth it - at least not without a lot more subtle and nuanced design and implementation work than I've already put into this PR. If someone wants to take that on in due course, good for them, but I don't want to merge something half-assed because I fear I'm just making a mess that's worse than the status quo.
I will carefully cherry-pick out the actual bugfix from this PR, but I'm abandoning the rest.
I've decided this may have been misguided. It's way too complicated and the correct behaviours to implement are not clear. Several dilemmas:
patch
util supports ignoring it. What's the right behaviour?What about trailing garbage after a hunk header, on the same line as it? Git heuristically generates such garbage to try to show the enclosing context (e.g. function declaration) that a hunk appears in - e.g. the
export function formatPatch(diff) {
here:if we don't support that, we're still not really letting people roundtrip their patches through a
parsePatch
/formatPatch
combo.formatPatch
do if someone passes it a structured patch object where they've manually set theleadingGarbage
to include file headers - i.e. to not be valid garbage? We should probably reject that somehow, but that further complicated the API.Not worth it - at least not without a lot more subtle and nuanced design and implementation work than I've already put into this PR. If someone wants to take that on in due course, good for them, but I don't want to merge something half-assed because I fear I'm just making a mess that's worse than the status quo.
I will carefully cherry-pick out the actual bugfix from this PR, but I'm abandoning the rest.