* [PATCH] mergetool--lib: refactor {diff,merge}_cmd logic
@ 2013-06-16 17:51 John Keeping
2013-06-17 5:31 ` David Aguilar
0 siblings, 1 reply; 3+ messages in thread
From: John Keeping @ 2013-06-16 17:51 UTC (permalink / raw)
To: git; +Cc: David Aguilar, 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 <john@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
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] mergetool--lib: refactor {diff,merge}_cmd logic
2013-06-16 17:51 [PATCH] mergetool--lib: refactor {diff,merge}_cmd logic John Keeping
@ 2013-06-17 5:31 ` David Aguilar
2013-06-17 20:13 ` Junio C Hamano
0 siblings, 1 reply; 3+ messages in thread
From: David Aguilar @ 2013-06-17 5:31 UTC (permalink / raw)
To: John Keeping; +Cc: Git Mailing List
On Sun, Jun 16, 2013 at 10:51 AM, John Keeping <john@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 <john@keeping.me.uk>
> ---
This is a nice cleanup.
Acked-by: David Aguilar <davvid@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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] mergetool--lib: refactor {diff,merge}_cmd logic
2013-06-17 5:31 ` David Aguilar
@ 2013-06-17 20:13 ` Junio C Hamano
0 siblings, 0 replies; 3+ messages in thread
From: Junio C Hamano @ 2013-06-17 20:13 UTC (permalink / raw)
To: David Aguilar; +Cc: John Keeping, Git Mailing List
David Aguilar <davvid@gmail.com> writes:
> On Sun, Jun 16, 2013 at 10:51 AM, John Keeping <john@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 <john@keeping.me.uk>
>> ---
>
> This is a nice cleanup.
>
> Acked-by: David Aguilar <davvid@gmail.com>
Thanks, both.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2013-06-17 20:13 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-06-16 17:51 [PATCH] mergetool--lib: refactor {diff,merge}_cmd logic John Keeping
2013-06-17 5:31 ` David Aguilar
2013-06-17 20:13 ` Junio C Hamano
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
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 read-only IMAP folder(s) and NNTP newsgroup(s).