Re: [PATCH] mergetool--lib: refactor {diff,merge}_cmd logic

2013-06-17 Thread Junio C Hamano 
David Aguilar dav...@gmail.com writes:

 On Sun, Jun 16, 2013 at 10:51 AM, John Keeping j...@keeping.me.uk wrote:
 Instead of needing a wrapper to call the diff/merge command, simply
 provide the diff_cmd and merge_cmd functions for user-specified tools in
 the same way as we do for built-in tools.

 Signed-off-by: John Keeping j...@keeping.me.uk
 ---

 This is a nice cleanup.

 Acked-by: David Aguilar dav...@gmail.com

Thanks, both.
--
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

[PATCH] mergetool--lib: refactor {diff,merge}_cmd logic

2013-06-16 Thread John Keeping 
Instead of needing a wrapper to call the diff/merge command, simply
provide the diff_cmd and merge_cmd functions for user-specified tools in
the same way as we do for built-in tools.

Signed-off-by: John Keeping j...@keeping.me.uk
---
 git-mergetool--lib.sh | 82 ++-
 1 file changed, 35 insertions(+), 47 deletions(-)

diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index e338be5..6a72106 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -114,6 +114,33 @@ valid_tool () {
test -n $cmd
 }
 
+setup_user_tool () {
+   merge_tool_cmd=$(get_merge_tool_cmd $tool)
+   test -n $merge_tool_cmd || return 1
+
+   diff_cmd () {
+   ( eval $merge_tool_cmd )
+   status=$?
+   return $status
+   }
+
+   merge_cmd () {
+   trust_exit_code=$(git config --bool \
+   mergetool.$1.trustExitCode || echo false)
+   if test $trust_exit_code = false
+   then
+   touch $BACKUP
+   ( eval $merge_tool_cmd )
+   status=$?
+   check_unchanged
+   else
+   ( eval $merge_tool_cmd )
+   status=$?
+   fi
+   return $status
+   }
+}
+
 setup_tool () {
tool=$1
 
@@ -142,15 +169,15 @@ setup_tool () {
 
if ! test -f $MERGE_TOOLS_DIR/$tool
then
-   # Use a special return code for this case since we want to
-   # source defaults even when an explicit tool path is
-   # configured since the user can use that to override the
-   # default path in the scriptlet.
-   return 2
+   setup_user_tool
+   return $?
fi
 
# Load the redefined functions
. $MERGE_TOOLS_DIR/$tool
+   # Now let the user override the default command for the tool.  If
+   # they have not done so then this will return 1 which we ignore.
+   setup_user_tool
 
if merge_mode  ! can_merge
then
@@ -187,20 +214,7 @@ run_merge_tool () {
status=0
 
# Bring tool-specific functions into scope
-   setup_tool $1
-   exitcode=$?
-   case $exitcode in
-   0)
-   :
-   ;;
-   2)
-   # The configured tool is not a built-in tool.
-   test -n $merge_tool_path || return 1
-   ;;
-   *)
-   return $exitcode
-   ;;
-   esac
+   setup_tool $1 || return 1
 
if merge_mode
then
@@ -213,38 +227,12 @@ run_merge_tool () {
 
 # Run a either a configured or built-in diff tool
 run_diff_cmd () {
-   merge_tool_cmd=$(get_merge_tool_cmd $1)
-   if test -n $merge_tool_cmd
-   then
-   ( eval $merge_tool_cmd )
-   status=$?
-   return $status
-   else
-   diff_cmd $1
-   fi
+   diff_cmd $1
 }
 
 # Run a either a configured or built-in merge tool
 run_merge_cmd () {
-   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)
-   if test $trust_exit_code = false
-   then
-   touch $BACKUP
-   ( eval $merge_tool_cmd )
-   status=$?
-   check_unchanged
-   else
-   ( eval $merge_tool_cmd )
-   status=$?
-   fi
-   return $status
-   else
-   merge_cmd $1
-   fi
+   merge_cmd $1
 }
 
 list_merge_tool_candidates () {
-- 
1.8.3.779.g691e267

--
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

Re: [PATCH] mergetool--lib: refactor {diff,merge}_cmd logic

2013-06-16 Thread David Aguilar 
On Sun, Jun 16, 2013 at 10:51 AM, John Keeping j...@keeping.me.uk wrote:
 Instead of needing a wrapper to call the diff/merge command, simply
 provide the diff_cmd and merge_cmd functions for user-specified tools in
 the same way as we do for built-in tools.

 Signed-off-by: John Keeping j...@keeping.me.uk
 ---

This is a nice cleanup.

Acked-by: David Aguilar dav...@gmail.com


  git-mergetool--lib.sh | 82 
 ++-
  1 file changed, 35 insertions(+), 47 deletions(-)

 diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
 index e338be5..6a72106 100644
 --- a/git-mergetool--lib.sh
 +++ b/git-mergetool--lib.sh
 @@ -114,6 +114,33 @@ valid_tool () {
 test -n $cmd
  }

 +setup_user_tool () {
 +   merge_tool_cmd=$(get_merge_tool_cmd $tool)
 +   test -n $merge_tool_cmd || return 1
 +
 +   diff_cmd () {
 +   ( eval $merge_tool_cmd )
 +   status=$?
 +   return $status
 +   }
 +
 +   merge_cmd () {
 +   trust_exit_code=$(git config --bool \
 +   mergetool.$1.trustExitCode || echo false)
 +   if test $trust_exit_code = false
 +   then
 +   touch $BACKUP
 +   ( eval $merge_tool_cmd )
 +   status=$?
 +   check_unchanged
 +   else
 +   ( eval $merge_tool_cmd )
 +   status=$?
 +   fi
 +   return $status
 +   }
 +}
 +
  setup_tool () {
 tool=$1

 @@ -142,15 +169,15 @@ setup_tool () {

 if ! test -f $MERGE_TOOLS_DIR/$tool
 then
 -   # Use a special return code for this case since we want to
 -   # source defaults even when an explicit tool path is
 -   # configured since the user can use that to override the
 -   # default path in the scriptlet.
 -   return 2
 +   setup_user_tool
 +   return $?
 fi

 # Load the redefined functions
 . $MERGE_TOOLS_DIR/$tool
 +   # Now let the user override the default command for the tool.  If
 +   # they have not done so then this will return 1 which we ignore.
 +   setup_user_tool

 if merge_mode  ! can_merge
 then
 @@ -187,20 +214,7 @@ run_merge_tool () {
 status=0

 # Bring tool-specific functions into scope
 -   setup_tool $1
 -   exitcode=$?
 -   case $exitcode in
 -   0)
 -   :
 -   ;;
 -   2)
 -   # The configured tool is not a built-in tool.
 -   test -n $merge_tool_path || return 1
 -   ;;
 -   *)
 -   return $exitcode
 -   ;;
 -   esac
 +   setup_tool $1 || return 1

 if merge_mode
 then
 @@ -213,38 +227,12 @@ run_merge_tool () {

  # Run a either a configured or built-in diff tool
  run_diff_cmd () {
 -   merge_tool_cmd=$(get_merge_tool_cmd $1)
 -   if test -n $merge_tool_cmd
 -   then
 -   ( eval $merge_tool_cmd )
 -   status=$?
 -   return $status
 -   else
 -   diff_cmd $1
 -   fi
 +   diff_cmd $1
  }

  # Run a either a configured or built-in merge tool
  run_merge_cmd () {
 -   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)
 -   if test $trust_exit_code = false
 -   then
 -   touch $BACKUP
 -   ( eval $merge_tool_cmd )
 -   status=$?
 -   check_unchanged
 -   else
 -   ( eval $merge_tool_cmd )
 -   status=$?
 -   fi
 -   return $status
 -   else
 -   merge_cmd $1
 -   fi
 +   merge_cmd $1
  }

  list_merge_tool_candidates () {
 --
 1.8.3.779.g691e267




--
David
--
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