git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: David Aguilar <davvid@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, John Keeping <john@keeping.me.uk>
Subject: [PATCH v3 1/4] mergetool--lib: simplify command expressions
Date: Tue, 29 Jan 2013 22:20:54 -0800	[thread overview]
Message-ID: <1359526854-25132-1-git-send-email-davvid@gmail.com> (raw)
In-Reply-To: <7vip6foc9m.fsf@alter.siamese.dyndns.org>

Update variable assignments to always use $(command "$arg")
in their RHS instead of "$(command "$arg")" as the latter
is harder to read.  Make get_merge_tool_cmd() simpler by
avoiding "echo" and $(command) substitutions completely.

Signed-off-by: David Aguilar <davvid@gmail.com>
---
This is a replacement patch for what's currently in pu to
fix the empty "test -z" expression.

 git-mergetool--lib.sh | 42 ++++++++++++++++++------------------------
 1 file changed, 18 insertions(+), 24 deletions(-)

diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index 1d0fb12..1ff6d38 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -32,17 +32,10 @@ check_unchanged () {
 	fi
 }
 
-valid_tool_config () {
-	if test -n "$(get_merge_tool_cmd "$1")"
-	then
-		return 0
-	else
-		return 1
-	fi
-}
-
 valid_tool () {
-	setup_tool "$1" || valid_tool_config "$1"
+	setup_tool "$1" && return 0
+	cmd=$(get_merge_tool_cmd "$1")
+	test -n "$cmd"
 }
 
 setup_tool () {
@@ -96,14 +89,13 @@ setup_tool () {
 }
 
 get_merge_tool_cmd () {
-	# Prints the custom command for a merge tool
 	merge_tool="$1"
 	if diff_mode
 	then
-		echo "$(git config difftool.$merge_tool.cmd ||
-			git config mergetool.$merge_tool.cmd)"
+		git config "difftool.$merge_tool.cmd" ||
+		git config "mergetool.$merge_tool.cmd"
 	else
-		echo "$(git config mergetool.$merge_tool.cmd)"
+		git config "mergetool.$merge_tool.cmd"
 	fi
 }
 
@@ -114,7 +106,7 @@ run_merge_tool () {
 	GIT_PREFIX=${GIT_PREFIX:-.}
 	export GIT_PREFIX
 
-	merge_tool_path="$(get_merge_tool_path "$1")" || exit
+	merge_tool_path=$(get_merge_tool_path "$1") || exit
 	base_present="$2"
 	status=0
 
@@ -145,7 +137,7 @@ run_merge_tool () {
 
 # Run a either a configured or built-in diff tool
 run_diff_cmd () {
-	merge_tool_cmd="$(get_merge_tool_cmd "$1")"
+	merge_tool_cmd=$(get_merge_tool_cmd "$1")
 	if test -n "$merge_tool_cmd"
 	then
 		( eval $merge_tool_cmd )
@@ -158,11 +150,11 @@ run_diff_cmd () {
 
 # Run a either a configured or built-in merge tool
 run_merge_cmd () {
-	merge_tool_cmd="$(get_merge_tool_cmd "$1")"
+	merge_tool_cmd=$(get_merge_tool_cmd "$1")
 	if test -n "$merge_tool_cmd"
 	then
-		trust_exit_code="$(git config --bool \
-			mergetool."$1".trustExitCode || echo false)"
+		trust_exit_code=$(git config --bool \
+			"mergetool.$1.trustExitCode" || echo false)
 		if test "$trust_exit_code" = "false"
 		then
 			touch "$BACKUP"
@@ -253,7 +245,7 @@ guess_merge_tool () {
 	# Loop over each candidate and stop when a valid merge tool is found.
 	for i in $tools
 	do
-		merge_tool_path="$(translate_merge_tool_path "$i")"
+		merge_tool_path=$(translate_merge_tool_path "$i")
 		if type "$merge_tool_path" >/dev/null 2>&1
 		then
 			echo "$i"
@@ -300,9 +292,11 @@ get_merge_tool_path () {
 	fi
 	if test -z "$merge_tool_path"
 	then
-		merge_tool_path="$(translate_merge_tool_path "$merge_tool")"
+		merge_tool_path=$(translate_merge_tool_path "$merge_tool")
 	fi
-	if test -z "$(get_merge_tool_cmd "$merge_tool")" &&
+
+	merge_tool_cmd=$(get_merge_tool_cmd "$merge_tool")
+	if test -z "$merge_tool_cmd" &&
 		! type "$merge_tool_path" >/dev/null 2>&1
 	then
 		echo >&2 "The $TOOL_MODE tool $merge_tool is not available as"\
@@ -314,11 +308,11 @@ get_merge_tool_path () {
 
 get_merge_tool () {
 	# Check if a merge tool has been configured
-	merge_tool="$(get_configured_merge_tool)"
+	merge_tool=$(get_configured_merge_tool)
 	# Try to guess an appropriate merge tool if no tool has been set.
 	if test -z "$merge_tool"
 	then
-		merge_tool="$(guess_merge_tool)" || exit
+		merge_tool=$(guess_merge_tool) || exit
 	fi
 	echo "$merge_tool"
 }
-- 
1.8.0.9.g3370a50

  reply	other threads:[~2013-01-30  6:21 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
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         ` David Aguilar [this message]
2013-01-30  7:04           ` [PATCH v3 1/4] mergetool--lib: simplify " 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

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=1359526854-25132-1-git-send-email-davvid@gmail.com \
    --to=davvid@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=john@keeping.me.uk \
    /path/to/YOUR_REPLY

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

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
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;
as well as URLs for NNTP newsgroup(s).