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

public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed

From: Junio C Hamano <gitster@pobox•com>
To: John Keeping <john@keeping•me.uk>
Cc: David Aguilar <davvid@gmail•com>, git@vger•kernel.org
Subject: Re: [PATCH v2 3/4] mergetool--lib: Add functions for finding available tools
Date: Tue, 29 Jan 2013 19:08:56 -0800	[thread overview]
Message-ID: <7vwquvmkon.fsf@alter.siamese.dyndns.org> (raw)
In-Reply-To: <20130129230607.GG1342@serenity.lan> (John Keeping's message of "Tue, 29 Jan 2013 23:06:08 +0000")

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

> 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

next prev parent reply	other threads:[~2013-01-30  3:09 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-01-28  0:52 [PATCH v2 0/4] Auto-generate mergetool lists David Aguilar
2013-01-28  0:52 ` [PATCH v2 1/4] mergetool--lib: Simplify command expressions David Aguilar
2013-01-28  0:52   ` [PATCH v2 2/4] mergetool--lib: Improve the help text in guess_merge_tool() David Aguilar
2013-01-28  0:52     ` [PATCH v2 3/4] mergetool--lib: Add functions for finding available tools David Aguilar
2013-01-28  0:52       ` [PATCH v2 4/4] doc: Generate a list of valid merge tools David Aguilar
2013-01-28  2:14         ` Junio C Hamano
2013-01-28 19:37       ` [PATCH v2 3/4] mergetool--lib: Add functions for finding available tools John Keeping
2013-01-28 20:36         ` Junio C Hamano
2013-01-29 19:48       ` John Keeping
2013-01-29 20:22         ` Junio C Hamano
2013-01-29 22:27           ` David Aguilar
2013-01-29 22:55             ` Junio C Hamano
2013-01-29 23:06               ` John Keeping
2013-01-30  3:08                 ` Junio C Hamano [this message]
2013-01-30  3:34                   ` Junio C Hamano
2013-01-29 23:02             ` John Keeping
2013-01-29 19:22   ` [PATCH v2 1/4] mergetool--lib: Simplify command expressions John Keeping
2013-01-29 22:09     ` David Aguilar
2013-01-29 22:27       ` Junio C Hamano
2013-01-30  6:20         ` [PATCH v3 1/4] mergetool--lib: simplify " David Aguilar
2013-01-30  7:04           ` Junio C Hamano
2013-01-28  2:08 ` [PATCH v2 0/4] Auto-generate mergetool lists Junio C Hamano
2013-01-28  2:21   ` David Aguilar
2013-01-28  2:27     ` Junio C Hamano
2013-01-28  2:41       ` David Aguilar
2013-01-28  2:53         ` Junio C Hamano
2013-01-28 21:01         ` John Keeping
2013-01-28 21:50           ` Junio C Hamano
2013-01-28 22:21             ` John Keeping
2013-01-29 11:56               ` Joachim Schmitz
2013-01-29 12:09                 ` John Keeping
2013-01-29 16:15                   ` Junio C Hamano
2013-01-29 16:46                     ` John Keeping
2013-01-28  8:20 ` Philip Oakley
2013-01-28  9:16   ` David Aguilar
2013-01-28 21:19     ` Philip Oakley

find likely ancestor, descendant, or conflicting patches for this message:
( dfblob:135da96 dfblob:79cbdc7 )
 OR (
bs:"[SQUASH] mergetools: tweak show_tool_names and its users" )
	(help)

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7vwquvmkon.fsf@alter.siamese.dyndns.org \
    --to=gitster@pobox$(echo .)com \
    --cc=davvid@gmail$(echo .)com \
    --cc=git@vger$(echo .)kernel.org \
    --cc=john@keeping$(echo .)me.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

Be sure your reply has a Subject: header at the top and a blank line before the message body.

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox