From e00e13e2aa5e94dc77dedb8f2026198300c7c8fe Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Fri, 21 Nov 2014 11:17:57 -0800 Subject: [PATCH 1/5] mergetool--lib: remove no-op assignment to $status from setup_user_tool Even though setup_user_tool assigns the exit status from "eval $merge_tool_cmd" to $status, the variable is overwritten by the function it calls next, check_unchanged, without ever getting looked at by anybody. And "return $status" at the end of this function returns the value check_unchanged assigned to it (which is the same as the value the function returns). Which makes the assignment a no-op. Remove it. Signed-off-by: Junio C Hamano --- git-mergetool--lib.sh | 1 - 1 file changed, 1 deletion(-) diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh index 2b6635130a..3e06389136 100644 --- a/git-mergetool--lib.sh +++ b/git-mergetool--lib.sh @@ -130,7 +130,6 @@ setup_user_tool () { then touch "$BACKUP" ( eval $merge_tool_cmd ) - status=$? check_unchanged else ( eval $merge_tool_cmd ) From 1b6a53431c0ffba4fb2428ef09018a5e889f52f8 Mon Sep 17 00:00:00 2001 From: David Aguilar Date: Thu, 20 Nov 2014 17:20:27 -0800 Subject: [PATCH 2/5] mergetool--lib: remove use of $status global Remove return statements and rework check_unchanged() so that the exit status from the last evaluated expression bubbles up to the callers. Signed-off-by: David Aguilar Signed-off-by: Junio C Hamano --- git-mergetool--lib.sh | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh index 3e06389136..fe61e89f31 100644 --- a/git-mergetool--lib.sh +++ b/git-mergetool--lib.sh @@ -92,7 +92,7 @@ translate_merge_tool_path () { check_unchanged () { if test "$MERGED" -nt "$BACKUP" then - status=0 + return 0 else while true do @@ -100,8 +100,8 @@ check_unchanged () { printf "Was the merge successful? [y/n] " read answer || return 1 case "$answer" in - y*|Y*) status=0; break ;; - n*|N*) status=1; break ;; + y*|Y*) return 0 ;; + n*|N*) return 1 ;; esac done fi @@ -119,8 +119,6 @@ setup_user_tool () { diff_cmd () { ( eval $merge_tool_cmd ) - status=$? - return $status } merge_cmd () { @@ -133,9 +131,7 @@ setup_user_tool () { check_unchanged else ( eval $merge_tool_cmd ) - status=$? fi - return $status } } @@ -152,13 +148,11 @@ setup_tool () { } diff_cmd () { - status=1 - return $status + return 1 } merge_cmd () { - status=1 - return $status + return 1 } translate_merge_tool_path () { @@ -209,7 +203,6 @@ run_merge_tool () { merge_tool_path=$(get_merge_tool_path "$1") || exit base_present="$2" - status=0 # Bring tool-specific functions into scope setup_tool "$1" || return 1 @@ -220,8 +213,6 @@ run_merge_tool () { else run_diff_cmd "$1" fi - status=$? - return $status } # Run a either a configured or built-in diff tool From c41d3fedd8a5dec2c2c991822b70b91c50ca047e Mon Sep 17 00:00:00 2001 From: David Aguilar Date: Thu, 20 Nov 2014 17:20:28 -0800 Subject: [PATCH 3/5] difftool--helper: add explicit exit statement git-difftool--helper returns a zero exit status unless --trust-exit-code is in effect. Add an explicit exit statement to make this clearer. Signed-off-by: David Aguilar Signed-off-by: Junio C Hamano --- git-difftool--helper.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/git-difftool--helper.sh b/git-difftool--helper.sh index d4fb6dfe13..2b11b1d6fe 100755 --- a/git-difftool--helper.sh +++ b/git-difftool--helper.sh @@ -94,3 +94,5 @@ else shift 7 done fi + +exit 0 From 98a260220c9b464dbefce1a23e4c89ed7b6ccb4d Mon Sep 17 00:00:00 2001 From: David Aguilar Date: Thu, 20 Nov 2014 17:20:29 -0800 Subject: [PATCH 4/5] mergetool: simplify conditionals Combine the $last_status checks into a single conditional. Replace $last_status and $rollup_status with a single variable. Signed-off-by: David Aguilar Signed-off-by: Junio C Hamano --- git-mergetool.sh | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/git-mergetool.sh b/git-mergetool.sh index ff050e58ff..d20581c15c 100755 --- a/git-mergetool.sh +++ b/git-mergetool.sh @@ -426,8 +426,6 @@ fi merge_keep_backup="$(git config --bool mergetool.keepBackup || echo true)" merge_keep_temporaries="$(git config --bool mergetool.keepTemporaries || echo false)" -last_status=0 -rollup_status=0 files= if test $# -eq 0 @@ -455,19 +453,15 @@ printf "%s\n" "$files" IFS=' ' +rc=0 for i in $files do - if test $last_status -ne 0 - then - prompt_after_failed_merge || exit 1 - fi printf "\n" - merge_file "$i" - last_status=$? - if test $last_status -ne 0 + if ! merge_file "$i" then - rollup_status=1 + rc=1 + prompt_after_failed_merge || exit 1 fi done -exit $rollup_status +exit $rc From 1e86d5b11dcf31adf11363caa1e96a94612b16f4 Mon Sep 17 00:00:00 2001 From: David Aguilar Date: Fri, 21 Nov 2014 01:03:10 -0800 Subject: [PATCH 5/5] mergetools: stop setting $status in merge_cmd() No callers rely on $status so there's don't need to set it during merge_cmd() for diffmerge, emerge, and kdiff3. Signed-off-by: David Aguilar Signed-off-by: Junio C Hamano --- mergetools/diffmerge | 1 - mergetools/emerge | 1 - mergetools/kdiff3 | 1 - 3 files changed, 3 deletions(-) diff --git a/mergetools/diffmerge b/mergetools/diffmerge index 85ac720157..f138cb4e73 100644 --- a/mergetools/diffmerge +++ b/mergetools/diffmerge @@ -11,5 +11,4 @@ merge_cmd () { "$merge_tool_path" --merge \ --result="$MERGED" "$LOCAL" "$REMOTE" fi - status=$? } diff --git a/mergetools/emerge b/mergetools/emerge index f96d9e550a..7b895fdb1f 100644 --- a/mergetools/emerge +++ b/mergetools/emerge @@ -15,7 +15,6 @@ merge_cmd () { "$LOCAL" "$REMOTE" \ "$(basename "$MERGED")" fi - status=$? } translate_merge_tool_path() { diff --git a/mergetools/kdiff3 b/mergetools/kdiff3 index a30034f116..793d1293b1 100644 --- a/mergetools/kdiff3 +++ b/mergetools/kdiff3 @@ -20,5 +20,4 @@ merge_cmd () { -o "$MERGED" "$LOCAL" "$REMOTE" \ >/dev/null 2>&1 fi - status=$? }