Re: [PATCH 3/3] Add -k/--keep-going option to mergetool

[Charles Bailey <charles@hashpling.org>]

Jeff King wrote:

>>  Documentation/config.txt        |    4 +++
>>  Documentation/git-mergetool.txt |   12 +++++++++-
>>  git-mergetool.sh                |   46
++++++++++++++++++++++++++++++--------
>>  3 files changed, 51 insertions(+), 11 deletions(-)
>
> This patch in particular changes some of the innards of mergetool, and
> while the changes look good to me via reading, I would feel even more
> comfortable if there were some tests (it looks like your previous t7610
> gives at least a basic sanity check, but it might be nice to test to
> make sure we detect merge failures, too).

I had a little difficulty thinking of some decently robust tests. git
mergetool is, by its nature, more interactive than many git commands.

With --no-prompt it should be easier to make some tests that don't just
hang when it goes wrong. I'll have a further think on this.

>> [...]
>> -	exit 1
>> +	cleanup_temp_files
>> +	return 1
>
> One of these is not like the others. Is there a reason to add a
> cleanup_temp_files here (and if so, should it be noted in the commit
> message, and/or be in a separate commit?).

Ah yes, it's definitely worthy of at least a comment. This is something
that I changed right at the end of testing and I should really have made
it into a separate commit.

Previously, if you aborted a merge, you were left with the
base/local/remote temporaries for the merge that you aborted.

To be honest, I found this a little irritating. The base, local and
remote temporaries are inputs so are accessible from slots 1,2 and 3 of
the index, and any intermediate output will be in the target file. You
can git clean, but if you have other temporaries you need to keep, you
end up having to manually clean them up in any case.

With --keep-going, the problem is compounded. If you make several passes
through a list of complex merges you end up with fistfuls of these
temporary trios in your working tree. It goes from slightly annoying to
very irritating.

Charles.