On Fri, Feb 16, 2024 at 10:12 AM Junio C Hamano <gitster@xxxxxxxxx> wrote: > > Patrick Steinhardt <ps@xxxxxx> writes: > > > The `--trust-exit-code` option for git-diff-tool(1) was introduced via > > 2b52123fcf (difftool: add support for --trust-exit-code, 2014-10-26). > > When set, it makes us return the exit code of the invoked diff tool when > > diffing multiple files. This patch didn't change the code path where > > `--dir-diff` was passed because we already returned the exit code of the > > diff tool unconditionally in that case. > > > > This was changed a month later via c41d3fedd8 (difftool--helper: add > > explicit exit statement, 2014-11-20), where an explicit `exit 0` was > > added to the end of git-difftool--helper.sh. While the stated intent of > > that commit was merely a cleanup, it had the consequence that we now > > to ignore the exit code of the diff tool when `--dir-diff` was set. This > > change in behaviour is thus very likely an unintended side effect of > > this patch. > > > > Now there are two ways to fix this: > > > > - We can either restore the original behaviour, which unconditionally > > returned the exit code of the diffing tool when `--dir-diff` is > > passed. > > > > - Or we can make the `--dir-diff` case respect the `--trust-exit-code` > > flag. > > > > The fact that we have been ignoring exit codes for 7 years by now makes > > me rather lean towards the latter option. Furthermore, respecting the > > flag in one case but not the other would needlessly make the user > > interface more complex. > > > > Fix the bug so that we also honor `--trust-exit-code` for dir diffs and > > adjust the documentation accordingly. > > > > Reported-by: Jean-Rémy Falleri <jr.falleri@xxxxxxxxx> > > Signed-off-by: Patrick Steinhardt <ps@xxxxxx> > > --- > > Sounds sensible. > > The last time David was on list seems to be in April 2023; just in > case let's CC him for an Ack (or something else). Thanks! I'm still lurking around. I've been meaning to make some Git-adjacent announcements and contributions soon.. Until then: Acked-by: David Aguilar <davvid@xxxxxxxxx> -- David
