Re: [PATCH v2 3/4] mergetool--lib: Add functions for finding available tools

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 30, 2013, 03:08 UTC
Message-ID: <7vwquvmkon.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20130129230607.GG1342@serenity.lan>

John Keeping <john@keeping.me.uk> writes:

Show 11 quoted lines

> On Tue, Jan 29, 2013 at 02:55:26PM -0800, Junio C Hamano wrote:
> ...
>> I can work with John to get this part into a shape to support his
>> extended use sometime toward the end of this week, by which time
>> hopefully you have some time to comment on the result.  John, how
>> does that sound?
>
> My email crossed with yours - that sounds good to me.  If
> da/mergetool-docs is in a reasonable state by tomorrow evening (GMT) I
> should be able to have a look at it then - if not I'm happy to hold off
> longer.

Heh, I actually was hoping that you will send in a replacement for David's patch ;-)

Here is what I will squash into the one we have been discussing. In a few hours, I expect I'll be able to push this out in the 'pu' branch.

-- >8 --
From: Junio C Hamano <gitster@pobox.com>
Date: Tue, 29 Jan 2013 18:57:55 -0800
Subject: [PATCH] [SQUASH] mergetools: tweak show_tool_names and its users

Use show_tool_names as a function to produce output, not as a function to compute a string. Indicate if any output was given with its return status, so that the caller can say "if it didn't give any output, I'll say this instead" easily.

To be squashed into the previous; no need to keep this log message.
---
 git-mergetool--lib.sh | 30 +++++++++++++++++-------------
 1 file changed, 17 insertions(+), 13 deletions(-)

diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index 135da96..79cbdc7 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -22,7 +22,7 @@ is_available () {
 show_tool_names () {
 	condition=${1:-true} per_line_prefix=${2:-} preamble=${3:-}
 
-	( cd "$MERGE_TOOLS_DIR" && ls -1 * ) |
+	( cd "$MERGE_TOOLS_DIR" && ls ) |
 	while read toolname
 	do
 		if setup_tool "$toolname" 2>/dev/null &&
@@ -36,6 +36,7 @@ show_tool_names () {
 			printf "%s%s\n" "$per_line_prefix" "$tool"
 		fi
 	done
+	test -n "$preamble"
 }
 
 diff_mode() {
@@ -236,27 +237,30 @@ list_merge_tool_candidates () {
 
 show_tool_help () {
 	tool_opt="'git ${TOOL_MODE}tool --tool-<tool>'"
-	available=$(show_tool_names 'mode_ok && is_available' '\t\t' \
-		"$tool_opt may be set to one of the following:")
-	unavailable=$(show_tool_names 'mode_ok && ! is_available' '\t\t' \
-		"The following tools are valid, but not currently available:")
-	if test -n "$available"
+
+	tab='	' av_shown= unav_shown=
+
+	if show_tool_names 'mode_ok && is_available' "$tab$tab" \
+		"$tool_opt may be set to one of the following:"
 	then
-		echo "$available"
+		av_shown=yes
 	else
 		echo "No suitable tool for 'git $cmd_name --tool=<tool>' found."
+		av_shown=no
 	fi
-	if test -n "$unavailable"
+
+	if show_tool_names 'mode_ok && ! is_available' "$tab$tab" \
+		"The following tools are valid, but not currently available:"
 	then
-		echo
-		echo "$unavailable"
+		unav_shown=yes
 	fi
-	if test -n "$unavailable$available"
-	then
+
+	case ",$av_shown,$unav_shown," in
+	*,yes,*)
 		echo
 		echo "Some of the tools listed above only work in a windowed"
 		echo "environment. If run in a terminal-only session, they will fail."
-	fi
+	esac
 	exit 0
 }

-- 
1.8.1.2.555.gedafe79