On 2024-05-27 07:19, Patrick Steinhardt wrote:
On Fri, May 24, 2024 at 02:46:43PM -0700, Junio C Hamano wrote:Patrick Steinhardt <ps@xxxxxx> writes: > On Thu, May 23, 2024 at 03:50:07PM -0700, Junio C Hamano wrote: > I also think that there's a bug here. The output from the above command > is: > ... > --- a/blorp > +++ b/blorp > @@ -1 +1 @@ > -fnorp > +fleep > Interdiff against v1: > diff --git a/blorp b/blorp > ... > > The diff is before the separator for the signature, and there is no > clear delimiter between the actual diff and the interdiff. Earlier Eric expressed concern about writing this out _after_ the mail signature mark "-- ", so the output deliberately goes before it. There is no need for any marker after the last line of the patch. "Interdiff against ..." is a clear enough delimiter. FWIW, the parsing of patches has always paid attention to the lengths recorded in @@ ... @@ hunk headers, and the parser notices where the run of ("diff --git a/... b/..." followed by a patch) ends and stops without problems. On the other hand, if you remove the line "+fleep" in the above example and try to feed it to "git apply", it would correctly notice that it failed to see the expected one line of postimage and complains (because it sees "Interdiff against..." when it expects to see a line that begins with a plus). So, I do not see any problem with the output from this cocde at all. Thanks for careful reading.The machine can cope alright. But I think that it's way harder to parse for a human if there is no clear visual delimiter between the diff and the interdiff. And "Interdiff" isn't quite ideal in my opinion because it is text, only, and may be quite easy to miss if it follows a long diff. The signature mark may not be ideal here as an indicator. Mail readers may hide signatures, color them differently or other stuff. But I think there should be some indicator here that visually highlights the fact that one section is ending and another section is starting. This could either be a newline, or the triple-dashes as we use in other places.
I agree about the need for having a distinctive separator.
