Re: [PATCH] mergetool--lib: refactor {diff,merge}_cmd logic
- From
David Aguilar <davvid@gmail.com>
- Date
- Jun 17, 2013, 05:31 UTC
- Message-ID
- <CAJDDKr5ewcYgtZn-oCQ5b=rGix5EALRdiSaNMatOwtP75e0h3g@mail.gmail.com>
- In-Reply-To
- <828585499e535649d14ef0ecd0f53403aefb10c2.1371405056.git.john@keeping.me.uk>
On Sun, Jun 16, 2013 at 10:51 AM, John Keeping <john@keeping.me.uk> wrote:
Show 6 quoted lines
> 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>
Show 128 quoted lines
> 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