From: SZEDER Gábor <szeder@ira.uka.de>
Date: Wed, 20 May 2015 11:09:29 GMT
Subject: Re: [PATCH v6 2/2] mergetools: add winmerge as a builtin tool
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>:

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

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