Re: [PATCH 1/2] Tidy up git mergetool's backup file behaviour and variable names

[Junio C Hamano <gitster@pobox.com>]

Charles Bailey <charles@hashpling.org> writes:

> Currently a backup pre-merge file with conflict markers is sometimes
> kept with a .orig extenstion and sometimes removed depending on the
> particular merge tool used.
>
> This patch makes the handling consistent across all merge tools and
> configurable via a new mergetool.keepBackup config variable
>
> Changed the merge file path variable to MERGED for consistency with the
> names of the merge temporary filename variables. This done with the
> intention of having these variables used by user scripts in a subsequent
> custom merge tool patch.

I would have preferred two separate patches, one s/path/MERGED/
and the other save/remove clean-up, which would be much easier
way to review, but this is what we have, so let's work on this
version.

> +mergetool.keepBackup::
> +	After performing a merge, the original file with conflict markers
> +	can be saved as a file with a `.orig` extension.  If this variable
> +	is set to `false` then this file is not preserved.
> +

s/$/  Defaults to true (i.e. keep the backup files)/.

We might also want a command line option to override the user's
usual default specified with this configuration but that can be
left for later rounds.

> @@ -112,11 +112,11 @@ resolve_deleted_merge () {
>  }
>  
>  check_unchanged () {
> -    if test "$path" -nt "$BACKUP" ; then
> +    if test "$MERGED" -nt "$BACKUP" ; then

I think this is the cause of your automated test sometimes
needing 1 sec sleep.  This check should perhaps be done with a
"!  cmp $MERGED $BACKUP >/dev/null", which would succeed if the
user's interaction with the backend tool touched the file.

The rest of the patch looked fine to me.

We might also want to add -y (assume "Yes" answer to "Did you
resolve it" question without actually asking) command line
option to the script, but that would be for later rounds.