On Fri, Jan 25, 2013 at 12:16:42PM -0800, Junio C Hamano wrote:
> John Keeping <j...@keeping.me.uk> writes:
> 
> > Actually, can we just change all of the above part of the loop to:
> >
> >     test "$tool" = defaults && continue
> >
> >     merge_tool_path=$(
> >             setup_tool "$tool" >/dev/null 2>&1 &&
> >             translate_merge_tool_path "$tool"
> >     ) || continue
> 
> Meaning "setup_tool ought to know which mode we are in and should
> fail if we are in merge mode and it does not support merging"?  That
> line of reasoning makes tons of sense to me, compared to this script
> implementing that logic for these scriptlets.

Yes, that's part of what setup_tool does.  It actually calls "exit" if
the "mode? && can_mode" test fails, which is why we need to call it in
the subshell.

I think this would get even better if we add a preparatory patch like
this, so we can just call setup_tool and then set merge_tool_path:

-- >8 --

diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index 888ae3e..4644cbf 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -67,11 +67,11 @@ setup_tool () {
        if merge_mode && ! can_merge
        then
                echo "error: '$tool' can not be used to resolve merges" >&2
-               exit 1
+               return 1
        elif diff_mode && ! can_diff
        then
                echo "error: '$tool' can only be used to resolve merges" >&2
-               exit 1
+               return 1
        fi
        return 0
 }
@@ -100,7 +100,7 @@ run_merge_tool () {
        status=0
 
        # Bring tool-specific functions into scope
-       setup_tool "$1"
+       setup_tool "$1" || return
 
        if merge_mode
        then

-- 8< --
 
> How/when does translate_merge_tool_path fail?

It doesn't - the "|| continue" is to catch errors from setup_tool.


John
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to