From: "René Scharfe" <l.s.r@web.de>
To: phillip.wood@dunelm.org.uk,
German Lashevich <german.lashevich@gmail.com>,
git@vger.kernel.org
Cc: Junio C Hamano <gitster@pobox.com>
Subject: Re: [PATCH 2/2] diff: fix --exit-code with external diff
Date: Sun, 12 May 2024 11:38:44 +0200 [thread overview]
Message-ID: <e97d91db-cbb5-43f5-84d0-289681620558@web.de> (raw)
In-Reply-To: <f3454199-2eed-4ca1-82db-7f1419b3084e@web.de>
Am 11.05.24 um 22:32 schrieb René Scharfe:
> Am 08.05.24 um 17:25 schrieb phillip.wood123@gmail.com:
>> Hi René
>>
>> On 06/05/2024 19:23, René Scharfe wrote:
>>> Am 05.05.24 um 17:25 schrieb Phillip Wood:
>>>> Merge strategies are expected to exit 0 on success, 1 when there are
>>>> conflicts and another non-zero value for other errors - it would be
>>>> nice to do something similar here where 1 means "there were
>>>> differences" but it is probably too late to do that without a config
>>>> value to indicate that we should trust the exit code.
>>> Right, such a diff command protocol v2 would not need to pipe the
>>> output through an inspection loop. Sounds like a good idea. It's
>>> unfortunate that it would increase the configuration surface, which is
>>> not in an acute need to expand. We could advertise the new option when
>>> dying due to the unsupported combination of --exit-code and external
>>> diff, but that's in equal parts helpful and obnoxious, I feel.
>>
>> Yes, diff dying would be annoying but the message would be useful.
>
> Having poked at it a bit more, I wonder if we need to add some further
> nuance/trick to avoid having to reject certain combinations of options.
>
> Currently external diffs can't signal content equality. That doesn't
> matter for trivial equality (where content and mode of the two files
> match), as this case is always handled by the diff machinery already.
> Only lossy comparisons (e.g. ignoring whitespace) even have the need to
> signal equality.
>
> If we (continue to) assume that external diffs are lossless then we
> don't need to change the code, just document that assumption. And add a
> way to specify lossy external diffs that can signal whether they found
> interesting differences, to address the originally reported shortcoming.
One more step in that direction: If we continue to use exit code 0 to
mean "notable changes present" and redefine 1 to mean "non-trivial
equality" instead of "fatal error" then we don't need to add any config
options.
A downside is that the exit codes of diff(1) have the opposite meaning.
Which is weird in itself: You say "give me a diff" and it answers "true,
I got nothing for you" or "false, I actually had to print that dang
thing". Inherited from cmp(1), I guess. Which I further guess got it
from bcmp(3)?
But we can't directly use diff(1) anyway because we pass either one or
six parameters instead of the two that it would expect. Our external
diff calling protocol is already special enough that we can freely
chose the meaning of exit codes.
Any other downsides? Am I missing something?
René
next prev parent reply other threads:[~2024-05-12 9:38 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-20 1:13 Possible git-diff bug when using exit-code with diff filters German Lashevich
2024-04-21 10:42 ` René Scharfe
2024-04-21 18:17 ` Junio C Hamano
2024-04-21 18:32 ` rsbecker
2024-04-21 19:09 ` Junio C Hamano
2024-04-21 20:18 ` rsbecker
2024-05-05 10:19 ` René Scharfe
2024-05-06 17:22 ` Junio C Hamano
2024-05-05 10:19 ` [PATCH 1/2] diff: report unmerged paths as changes in run_diff_cmd() René Scharfe
2024-05-05 10:20 ` [PATCH 2/2] diff: fix --exit-code with external diff René Scharfe
2024-05-05 15:25 ` Phillip Wood
2024-05-06 17:31 ` Junio C Hamano
2024-05-06 18:23 ` René Scharfe
2024-05-08 15:25 ` phillip.wood123
2024-05-11 20:32 ` René Scharfe
2024-05-12 9:38 ` René Scharfe [this message]
2024-06-05 8:31 ` [PATCH v2 0/3] " René Scharfe
2024-06-05 8:35 ` [PATCH v2 1/3] t4020: test exit code with external diffs René Scharfe
2024-06-05 8:36 ` [PATCH v2 2/3] userdiff: add and use struct external_diff René Scharfe
2024-06-05 8:38 ` [PATCH v2 3/3] diff: let external diffs report that changes are uninteresting René Scharfe
2024-06-06 6:39 ` Johannes Sixt
2024-06-06 8:28 ` René Scharfe
2024-06-06 15:49 ` Junio C Hamano
2024-06-06 9:48 ` Phillip Wood
2024-06-07 8:19 ` René Scharfe
2024-06-05 16:47 ` [PATCH v2 0/3] diff: fix --exit-code with external diff Junio C Hamano
2024-06-09 7:35 ` [PATCH v3 " René Scharfe
2024-06-09 7:38 ` [PATCH v3 1/3] t4020: test exit code with external diffs René Scharfe
2024-06-10 16:33 ` Junio C Hamano
2024-06-09 7:39 ` [PATCH v3 2/3] userdiff: add and use struct external_diff René Scharfe
2024-06-09 7:41 ` [PATCH v3 3/3] diff: let external diffs report that changes are uninteresting René Scharfe
2024-06-10 13:48 ` [PATCH v3 0/3] diff: fix --exit-code with external diff phillip.wood123
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=e97d91db-cbb5-43f5-84d0-289681620558@web.de \
--to=l.s.r@web.de \
--cc=german.lashevich@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=phillip.wood@dunelm.org.uk \
/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 read-only IMAP folder(s) and NNTP newsgroup(s).