Re: [PATCH v5 14/14] difftool/mergetool: refactor commands to use git-mergetool--lib

David Aguilar [off-list ref] writes:

This consolidates the common functionality from git-mergetool and
git-difftool--helper into a single git-mergetool--lib scriptlet.

+guess_merge_tool () {
+	tools="ecmerge"
+	if merge_mode; then
+		tools="tortoisemerge"
+	else
+		tools="kompare"
+	fi

The first assignment of ecmerge seems a no-op.

+	if test -n "$DISPLAY"; then
+		if test -n "$GNOME_DESKTOP_SESSION_ID" ; then
+			tools="meld opendiff kdiff3 tkdiff xxdiff $tools \
+				gvimdiff diffuse ecmerge"
+		else
+			tools="opendiff kdiff3 tkdiff xxdiff meld $tools \
+				gvimdiff diffuse ecmerge"
+		fi
+	fi
+	if echo "${VISUAL:-$EDITOR}" | grep emacs > /dev/null 2>&1; then
+		# $EDITOR is emacs so add emerge as a candidate
+		tools="$tools emerge vimdiff"
+	elif echo "${VISUAL:-$EDITOR}" | grep vim > /dev/null 2>&1; then
+		# $EDITOR is vim so add vimdiff as a candidate
+		tools="$tools vimdiff emerge"
+	else
+		tools="$tools emerge vimdiff"
+	fi
+	tools="$(echo "$tools" | sed -e 's/ 	*/ /g')"
+	echo >&2 "merge tool candidates: $tools"

Sorry, but I am not sure what this is doing.  Replace a SP followed by
zero or more HT with a single SP?  

Ahh, you are removing the HT in the indentation part?

I'd rather see the above written like this if that is the case:

+	if test -n "$DISPLAY"; then
+		if test -n "$GNOME_DESKTOP_SESSION_ID" ; then
+			tools="meld opendiff kdiff3 tkdiff xxdiff $tools"
+		else
+			tools="opendiff kdiff3 tkdiff xxdiff meld $tools"
+		fi
+ 		tools="$tools gvimdiff diffuse ecmerge"
+	fi

then you can lose the "echo | sed", no?