Re: [BUG] Regression in 'git mergetool --tool-help'

[Junio C Hamano <gitster@pobox.com>]

René Scharfe <l.s.r@web.de> writes:

> This still starts a subshell in the last line.  How about something
> like this?
>
> diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
> index 7225abd811..79d5ed1fa9 100644
> --- a/git-mergetool--lib.sh
> +++ b/git-mergetool--lib.sh
> @@ -46,11 +46,9 @@ show_tool_names () {
>  		while read scriptname
>  		do
>  			setup_tool "$scriptname" 2>/dev/null
> -			variants="$variants$(list_tool_variants)\n"
> -		done
> -		variants="$(echo "$variants" | sort | uniq)"
> -
> -		for toolname in $variants
> +			list_tool_variants
> +		done | sort | uniq |
> +		while read toolname
>  		do
>  			if setup_tool "$toolname" 2>/dev/null &&
>  				(eval "$condition" "$toolname")
>
> It requires setup_tool to be silent, though.

Another thing it depends on is that the side-effect of setup_tool in
the first loop does not matter in the end, as it now is done in the
upstream of a pipe.  It is a safe assumption to make (setup_tool is
called again in the later loop, so in the original it was called
twice), I think.

>> BTW, is `sort -u` not available everywhere?
>
> It's used by the function mergetool_find_win32_cmd in the same file
> and by several test scripts, so that shouldn't be a problem.

"sort -u" is safe; it is even in POSIX.

Having said that, when finding out how portable a construct we
already use is across the platforms we support, my recommendation is
to pretty much ignore what we do in a function or a file that we
know is only used by a single platform.  Stuff written for Windows,
for example, can assume that we use a particular implementation of
system-supplied tools and libraries.  The use of "sort -u" in
mergetool_find_win32_cmd can be legitimately justified with "We know
we use only GNU sed" and "we taught -u to our busybox sed already",
for example, if these statements are true.

Thanks.