From: David Aguilar <davvid@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, Sylvain Rabot <srabot@steek.com>,
Ramkumar Ramachandra <artagnon@gmail.com>,
K Gateway <kowloongateway1@gmail.com>,
Heiko Voigt <hvoigt@hvoigt.net>,
Sebastian Schuberth <sschuberth@gmail.com>,
Mike Schuld <mike.schuld@foundant.com>,
Stefan Kendall <stefankendall@gmail.com>
Subject: [PATCH] mergetool--lib: Allow custom commands to override built-ins
Date: Tue, 25 Sep 2012 00:48:11 -0700 [thread overview]
Message-ID: <1348559291-71739-1-git-send-email-davvid@gmail.com> (raw)
Allow users to override the default commands provided by the
mergetools/* scriptlets.
Users occasionally run into problems where they expect to be
able to override the built-in tool names. The documentation
does not explicitly mention that built-ins cannot be overridden,
so it's easy to assume that it should work.
Lift this restriction so that built-in tools are handled the
same way as user-configured tools. Add tests to guarantee this
behavior.
A nice benefit of this change is that it protects users from
having future versions of git trump their custom configuration
with a new built-in tool.
C.f.:
http://stackoverflow.com/questions/7435002/mergetool-from-gitconfig-being-ignored
http://thread.gmane.org/gmane.comp.version-control.msysgit/13188
http://thread.gmane.org/gmane.comp.version-control.git/148267
Signed-off-by: David Aguilar <davvid@gmail.com>
---
The Cc: list includes folks who were involved in the above threads.
Hopefully we can do without the workarounds now ;-)
git-mergetool--lib.sh | 40 ++++++++++++++++++++++++++++++++++++++--
mergetools/defaults | 28 ++--------------------------
t/t7610-mergetool.sh | 13 +++++++++++++
t/t7800-difftool.sh | 11 +++++++++++
4 files changed, 64 insertions(+), 28 deletions(-)
diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index 54cb708..f013a03 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -104,13 +104,49 @@ run_merge_tool () {
if merge_mode
then
- merge_cmd "$1"
+ run_merge_cmd "$1"
else
- diff_cmd "$1"
+ run_diff_cmd "$1"
fi
return $status
}
+# 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
+}
+
+# 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
+}
+
list_merge_tool_candidates () {
if merge_mode
then
diff --git a/mergetools/defaults b/mergetools/defaults
index 1d8f2a3..21e63ec 100644
--- a/mergetools/defaults
+++ b/mergetools/defaults
@@ -8,36 +8,12 @@ can_diff () {
}
diff_cmd () {
- merge_tool_cmd="$(get_merge_tool_cmd "$1")"
- if test -z "$merge_tool_cmd"
- then
- status=1
- break
- fi
- ( eval $merge_tool_cmd )
- status=$?
+ status=1
return $status
}
merge_cmd () {
- merge_tool_cmd="$(get_merge_tool_cmd "$1")"
- if test -z "$merge_tool_cmd"
- then
- status=1
- break
- fi
- 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
+ status=1
return $status
}
diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh
index 6fa0c70..bc38737 100755
--- a/t/t7610-mergetool.sh
+++ b/t/t7610-mergetool.sh
@@ -509,4 +509,17 @@ test_expect_success 'file with no base' '
git reset --hard master >/dev/null 2>&1
'
+test_expect_success 'custom commands override built-ins' '
+ git checkout -b test14 branch1 &&
+ git config mergetool.defaults.cmd "cat \"\$REMOTE\" >\"\$MERGED\"" &&
+ git config mergetool.defaults.trustExitCode true &&
+ test_must_fail git merge master &&
+ git mergetool --no-prompt --tool defaults -- both &&
+ echo master both added >expected &&
+ test_cmp both expected &&
+ git config --unset mergetool.defaults.cmd &&
+ git config --unset mergetool.defaults.trustExitCode &&
+ git reset --hard master >/dev/null 2>&1
+'
+
test_done
diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
index 9c3e997..eb1d3f8 100755
--- a/t/t7800-difftool.sh
+++ b/t/t7800-difftool.sh
@@ -76,6 +76,17 @@ test_expect_success PERL 'custom commands' '
test "$diff" = "branch"
'
+# Ensures that a custom difftool.<tool>.cmd overrides built-ins
+test_expect_success PERL 'custom commands override built-ins' '
+ restore_test_defaults &&
+ git config difftool.defaults.cmd "cat \$REMOTE" &&
+
+ diff=$(git difftool --tool defaults --no-prompt branch) &&
+ test "$diff" = "master" &&
+
+ git config --unset difftool.defaults.cmd
+'
+
# Ensures that git-difftool ignores bogus --tool values
test_expect_success PERL 'difftool ignores bad --tool values' '
diff=$(git difftool --no-prompt --tool=bad-tool branch)
--
1.7.12.1.402.g28d36ee
next reply other threads:[~2012-09-25 7:48 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-09-25 7:48 David Aguilar [this message]
2012-09-26 10:03 ` [PATCH] mergetool--lib: Allow custom commands to override built-ins Ramkumar Ramachandra
2012-09-26 14:06 ` Junio C Hamano
2012-09-26 18:31 ` David Aguilar
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1348559291-71739-1-git-send-email-davvid@gmail.com \
--to=davvid@gmail.com \
--cc=artagnon@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=hvoigt@hvoigt.net \
--cc=kowloongateway1@gmail.com \
--cc=mike.schuld@foundant.com \
--cc=srabot@steek.com \
--cc=sschuberth@gmail.com \
--cc=stefankendall@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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 NNTP newsgroup(s).