Re: [PATCH 1/2] mergetools: create mergetool_find_win32_cmd() helper function for winmerge

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Jacob Nisnevich <jacob.nisnevich@xxxxxxxxx> writes:

> Signed-off-by: Jacob Nisnevich <jacob.nisnevich@xxxxxxxxx>
> ---
>  git-mergetool--lib.sh | 25 +++++++++++++++++++++++++
>  mergetools/winmerge   | 21 +--------------------
>  2 files changed, 26 insertions(+), 20 deletions(-)
>
> diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
> index 54ac8e4..c5fa820 100644
> --- a/git-mergetool--lib.sh
> +++ b/git-mergetool--lib.sh
> @@ -372,3 +372,28 @@ get_merge_tool () {
>  	fi
>  	echo "$merge_tool"
>  }
> +
> +mergetool_find_win32_cmd () {
> +	executable=$1
> +	folder=$2
> +
> +	# Use executable.com if it exists in $PATH

s/executable.com/$executable/

> +	if type -p $executable >/dev/null 2>&1

This needs to be quoted,

	if type -p "$executable" >/dev/null 2>&1

> +	then
> +		printf '%s' $executable

Likewise.

> +		return
> +	fi
> +
> +	# Look for executable in the typical locations
> +	for directory in $(env | grep -Ei '^PROGRAM(FILES(\(X86\))?|W6432)=' |
> +		cut -d '=' -f 2- | sort -u)
> +	do
> +		if test -n "$directory" && test -x "$directory/$folder/$executable"
> +		then
> +			printf '%s' "$directory/$folder/$executable"
> +			return
> +		fi
> +	done
> +
> +	printf '%s' $executable

Likewise.

Other than these points, I do not see anything wrong in this patch.
Thanks.

By the way, "directory/folder/stuff" sounds somewhat strange, no?

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]