Re: [PATCH v6 2/2] mergetools: add winmerge as a builtin tool

[David Aguilar <davvid@gmail.com>]

[just wrapping up the unaswered questions in this thread]

On Wed, May 20, 2015 at 01:09:29PM +0200, SZEDER Gábor wrote:
> 
> Quoting David Aguilar <davvid@gmail.com>:
> 
> >+translate_merge_tool_path() {
> >+	# Use WinMergeU.exe if it exists in $PATH
> >+	if type -p WinMergeU.exe >/dev/null 2>&1
> >+	then
> >+		printf WinMergeU.exe
> >+		return
> >+	fi
> >+
> >+	# Look for WinMergeU.exe in the typical locations
> >+	winmerge_exe="WinMerge/WinMergeU.exe"
> 
> This variable is not used elsewhere, right?  Then you might want to
> mark it as local to make this clear.


"local" is a bash-ism, otherwise that'd be a good idea.


> >+	for directory in $(env | grep -Ei '^PROGRAM(FILES(\(X86\))?|W6432)=' |
> >+		cut -d '=' -f 2- | sort -u)
> >+	do
> >+		if test -n "$directory" && test -x "$directory/$winmerge_exe"
> >+		then
> >+			printf '%s' "$directory/$winmerge_exe"
> >+			return
> >+		fi
> >+	done
> >+
> >+	printf WinMergeU.exe
> 
> Please pardon my ignorance and curiosity, but what is the purpose of
> this last printf?
> It outputs the same as in the case when winmerge is in $PATH at the
> beginning of the function.  However, if we reach this printf, then
> winmerge is not in $PATH, so what will be executed?


This function maps what we call the tool (winmerge) to the actual executable.
That last printf provides the following behavior:

	$ git difftool -t winmerge HEAD~
	
	Viewing (1/1): 'mergetools/winmerge'
	Launch 'winmerge' [Y/n]:
	The diff tool winmerge is not available as 'WinMergeU.exe'
	fatal: external diff died, stopping at mergetools/winmerge

It ensures that the user sees 'WinMergeU.exe' in the error message.
That way the user can resolve the problem by e.g. adjusting their $PATH,
or realizing that they don't have WinMergeU.exe installed.
-- 
David