From: Junio C Hamano Date: Mon, 18 Feb 2008 03:45:42 GMT Subject: Re: [PATCH 1/2] Tidy up git mergetool's backup file behaviour and variable names Message-ID: <7vy79jsza1.fsf@gitster.siamese.dyndns.org> In-Reply-To: <9543203388d64839de78822efb538903fc15bf7f.1203251306.git.charles@hashpling.org> Charles Bailey 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.