Re: [PATCH 2/2] diff: fix --exit-code with external diff

["René Scharfe" <l.s.r@web.de>]

Am 05.05.24 um 17:25 schrieb Phillip Wood:
> On 05/05/2024 11:20, René Scharfe wrote:
>> Finally, capture the output of the external diff and only register a
>> change by setting found_changes if the program wrote anything.
>
> I worry that this will lead to regression reports like
> https://lore.kernel.org/git/CA+dzEBn108QoMA28f0nC8K21XT+Afua0V2Qv8XkR8rAeqUCCZw@mail.gmail.com/
> as the external diff will not see a terminal on stdout anymore.

So external diffs might no longer color their output if invoked using
the option --exit-code.  I assume this can be worked around by adding an
option like --color=always to the diff command.  This warrants a note in
the docs at the very least, though.

But thinking about it, the external diff could be a GUI program that
doesn't write to its stdout at all.  Or it could write something if the
files' contents match and stay silent if there are differences, weird as
that may be.

> I'm not really clear why we ignore the exit code of the external diff
> command.

Historical reasons, I guess.

> 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.

René