From: Patrick Steinhardt <ps@pks.im>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 2/2] format-patch: move range/inter diff at the end of a single patch output
Date: Mon, 27 May 2024 07:19:12 +0200 [thread overview]
Message-ID: <ZlQX0FmIsz2eFgsC@tanuki> (raw)
In-Reply-To: <xmqqo78ukhmk.fsf@gitster.g>
[-- Attachment #1: Type: text/plain, Size: 2193 bytes --]
On Fri, May 24, 2024 at 02:46:43PM -0700, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> 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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
next prev parent reply other threads:[~2024-05-27 5:19 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-23 22:50 [PATCH 0/2] give range-diff at the end of single patch output Junio C Hamano
2024-05-23 22:50 ` [PATCH 1/2] show_log: factor out interdiff/range-diff generation Junio C Hamano
2024-05-23 22:50 ` [PATCH 2/2] format-patch: move range/inter diff at the end of a single patch output Junio C Hamano
2024-05-24 11:14 ` Patrick Steinhardt
2024-05-24 21:46 ` Junio C Hamano
2024-05-27 5:19 ` Patrick Steinhardt [this message]
2024-05-27 12:59 ` Dragan Simic
2024-05-27 17:43 ` Junio C Hamano
2024-05-28 13:27 ` Patrick Steinhardt
2024-05-28 16:50 ` Junio C Hamano
2024-05-29 5:33 ` Patrick Steinhardt
2024-05-29 14:29 ` Junio C Hamano
2024-05-30 20:05 ` Dragan Simic
2024-05-24 23:02 ` [PATCH v2 " Junio C Hamano
2024-05-23 23:22 ` [PATCH 0/2] give range-diff at the end of " Dragan Simic
2024-05-23 23:25 ` Junio C Hamano
2024-05-23 23:35 ` Dragan Simic
2024-05-24 3:56 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZlQX0FmIsz2eFgsC@tanuki \
--to=ps@pks.im \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).