Re: [PATCH v9 4/5] mergetool: break setup_tool out into separate initialization function

[Johannes Sixt <j6t@kdbg.org>]

Am 28.12.20 um 20:29 schrieb Seth House:
> diff --git a/git-mergetool.sh b/git-mergetool.sh
> index e3c7d78d1d..929192d0f8 100755
> --- a/git-mergetool.sh
> +++ b/git-mergetool.sh
> @@ -334,6 +334,8 @@ merge_file () {
>  	checkout_staged_file 2 "$MERGED" "$LOCAL"
>  	checkout_staged_file 3 "$MERGED" "$REMOTE"
>  
> +	initialize_merge_tool "$merge_tool"

In my earlier review, I was not explicit about the lack of error
handling of this invocation, because I hoped that you would notice it
yourself. `initialize_merge_tool` does have a few failure modes via
`setup_tool` that are not really unlikely; ignoring errors would be
wrong, I think.

Before this change, the errors would be handled as part of the failing
`run_merge_tool` call at the end of function `merge_file`. But now
errors are ignored. Just appending `|| return` would not be appropriate
at this point because a lot has already happened before the call that
has to be rewound. Would it be possible to move the call above the
`mergetool_tmpdir_init` call, so that nothing has to be rewound?

> +
>  	if test "$(
>  		git config --get --bool "mergetool.$merge_tool.automerge" ||
>  		git config --get --bool "mergetool.automerge" ||
> 

-- Hannes