From: <rsbecker@nexbridge.com>
To: "'Junio C Hamano'" <gitster@pobox.com>, "'René Scharfe'" <l.s.r@web.de>
Cc: "'German Lashevich'" <german.lashevich@gmail.com>, <git@vger.kernel.org>
Subject: RE: Possible git-diff bug when using exit-code with diff filters
Date: Sun, 21 Apr 2024 14:32:30 -0400 [thread overview]
Message-ID: <03b501da941a$4896bdb0$d9c43910$@nexbridge.com> (raw)
In-Reply-To: <xmqqy1961sxf.fsf@gitster.g>
On Sunday, April 21, 2024 2:18 PM, Junio C Hamano wrote:
>René Scharfe <l.s.r@web.de> writes:
>
>> You can more easily reproduce it by setting the environment variable
>> GIT_EXTERNAL_DIFF or the configuration setting diff.external -- no
>> attributes needed.
>
>Indeed.
>
>A much simpler fix may be to declare that these two features are imcompatible and
>fail the execution upfront, instead of just silently ignoring one of the two options.
>
>As a person who is very much used to the external diff not contributing to the exit
>status (who also invented the external diff driver interface), I would be a wrong
>person to judge if such a simplified approach is desirable, of course, but just
>throwing it out as a food for thought.
My suggestion would be to keep with a priority approach, where GIT_EXTERNAL_DIFF overrides diff.external, assuming they set hold to the same specification (the git-config page implies they do) and GIT_EXTERNAL_DIFF overrides diff.external as I would expect. The behaviour of the two should be identical and definitely not in conflict. From my experience with this feature, and what I would prefer to preserve, is a formal separation between the diff engine and the underlying textconv. If the textconv processor fails in some way, the git diff should also fail, but the exit code should reflect that git diff failed, not the status of the textconv processor. If the external diff wrapper fails, the completion should be passed up since git would/should/could delegate the diff processing to the external engine. The only real completion code requirement I see is that 0 = no difference, 1 = difference exists, other is an indeterminate failure.
I might be restating the obvious, but this is an important capability, at least to me and my teams. We use this extensively for binary content diffs. It is also pretty important for structured document (e.g., MS-Word, RTF, PDF) and executable diffs that git does not handle out of the box.
--Randall
next prev parent reply other threads:[~2024-04-21 18:32 UTC|newest]
Thread overview: 13+ 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 [this message]
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
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='03b501da941a$4896bdb0$d9c43910$@nexbridge.com' \
--to=rsbecker@nexbridge.com \
--cc=german.lashevich@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=l.s.r@web.de \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.