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

From: SZEDER Gábor <szeder@ira.uka.de>
Date: May 20, 2015, 11:09 UTC
Message-ID: <20150520130929.Horde.vYwOuIDRpi6hr15rOUbW1w7@webmail.informatik.kit.edu>
In-Reply-To: <1432112843-973-2-git-send-email-davvid@gmail.com>

Quoting David Aguilar <davvid@gmail.com>:

Show 10 quoted lines

> +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.

Show 11 quoted lines

> +	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?

Gábor

> +}
> --
> 2.4.1.218.gc09a0e5