Re: [PATCH 1/2] Tidy up git mergetool's backup file behaviour and variable names
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:44:14
Charles Bailey [off-list ref] 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.
quoted hunk ↗ jump to hunk
@@ -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.