René Scharfe <l.s.r@xxxxxx> 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.
