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.
Patrick
Attachment:
signature.asc
Description: PGP signature
