[PATCH] difftool: add git-difftool to the list of commands

[PATCH] difftool: add git-difftool to the list of commands

[David Aguilar]

Signed-off-by: David Aguilar <davvid@gmail.com>
---
 command-list.txt |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/command-list.txt b/command-list.txt
index 3583a33..fb03a2e 100644
--- a/command-list.txt
+++ b/command-list.txt
@@ -33,6 +33,7 @@ git-diff                                mainporcelain common
 git-diff-files                          plumbinginterrogators
 git-diff-index                          plumbinginterrogators
 git-diff-tree                           plumbinginterrogators
+git-difftool                            ancillaryinterrogators
 git-fast-export				ancillarymanipulators
 git-fast-import				ancillarymanipulators
 git-fetch                               mainporcelain common
-- 
1.6.2.1.303.g63699

[PATCH] difftool: add various git-difftool tests

[David Aguilar]

t7800-difftool.sh tests the various command-line flags,
git-config variables, and environment settings supported by
git-difftool.

Signed-off-by: David Aguilar <davvid@gmail.com>
---
 t/t7800-difftool.sh |  139 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 139 insertions(+), 0 deletions(-)
 create mode 100755 t/t7800-difftool.sh

diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
new file mode 100755
index 0000000..c7cd2b1
--- /dev/null
+++ b/t/t7800-difftool.sh
@@ -0,0 +1,139 @@
+#!/bin/sh
+#
+# Copyright (c) 2009 David Aguilar
+#
+
+test_description='git-difftool
+
+Testing basic diff tool invocation
+'
+
+. ./test-lib.sh
+
+remove_config_vars()
+{
+	# Unset all config variables used by git-difftool
+	git config --unset diff.tool
+	git config --unset difftool.test-tool.cmd
+	git config --unset merge.tool
+	git config --unset mergetool.test-tool.cmd
+	return 0
+}
+
+restore_test_defaults()
+{
+	# Restores the test defaults used by several tests
+	remove_config_vars
+	unset GIT_DIFF_TOOL &&
+	unset GIT_MERGE_TOOL &&
+	unset GIT_DIFFTOOL_NO_PROMPT &&
+	git config diff.tool test-tool &&
+	git config difftool.test-tool.cmd "cat \$LOCAL"
+}
+
+# Create a file on master and change it on branch
+test_expect_success 'setup' '
+	echo master >file &&
+	git add file &&
+	git commit -m "added file" &&
+
+	git checkout -b branch master &&
+	echo branch >file &&
+	git commit -a -m "branch changed file" &&
+	git checkout master
+'
+
+# Configure a custom difftool.<tool>.cmd and use it
+test_expect_success 'custom commands' '
+	restore_test_defaults &&
+	git config difftool.test-tool.cmd "cat \$REMOTE" &&
+
+	diff=$(git difftool --no-prompt branch) &&
+	test "$diff" = "master" &&
+
+	restore_test_defaults &&
+	diff=$(git difftool --no-prompt branch) &&
+	test "$diff" = "branch"
+'
+
+# Ensures that git-difftool ignores bogus --tool values
+test_expect_success 'difftool ignores bad --tool values' '
+	diff=$(git difftool --no-prompt --tool=bogus-tool branch)
+	test "$?" = 1 &&
+	test "$diff" = ""
+'
+
+# Specify the diff tool using $GIT_DIFF_TOOL
+test_expect_success 'GIT_DIFF_TOOL variable' '
+	git config --unset diff.tool &&
+	GIT_DIFF_TOOL=test-tool &&
+	export GIT_DIFF_TOOL &&
+
+	diff=$(git difftool --no-prompt branch) &&
+	test "$diff" = "branch" &&
+
+	restore_test_defaults
+'
+
+# Test the $GIT_*_TOOL variables and ensure
+# that $GIT_DIFF_TOOL always wins unless --tool is specified
+test_expect_success 'GIT_DIFF_TOOL overrides' '
+	git config diff.tool bogus-tool &&
+	git config merge.tool bogus-tool &&
+
+	GIT_MERGE_TOOL=test-tool &&
+	export GIT_MERGE_TOOL &&
+	diff=$(git difftool --no-prompt branch) &&
+	test "$diff" = "branch" &&
+	unset GIT_MERGE_TOOL &&
+
+	GIT_MERGE_TOOL=bogus-tool &&
+	GIT_DIFF_TOOL=test-tool &&
+	export GIT_MERGE_TOOL &&
+	export GIT_DIFF_TOOL &&
+
+	diff=$(git difftool --no-prompt branch) &&
+	test "$diff" = "branch" &&
+
+	GIT_DIFF_TOOL=bogus-tool &&
+	export GIT_DIFF_TOOL &&
+
+	diff=$(git difftool --no-prompt --tool=test-tool branch) &&
+	test "$diff" = "branch" &&
+
+	restore_test_defaults
+'
+
+# Test that we don't have to pass --no-prompt to difftool
+# when $GIT_DIFFTOOL_NO_PROMPT is true
+test_expect_success 'GIT_DIFFTOOL_NO_PROMPT variable' '
+	GIT_DIFFTOOL_NO_PROMPT=true &&
+	export GIT_DIFFTOOL_NO_PROMPT &&
+
+	diff=$(git difftool branch) &&
+	test "$diff" = "branch" &&
+
+	restore_test_defaults
+'
+
+# git-difftool falls back to git-mergetool config variables
+# so test that behavior here
+test_expect_success 'difftool + mergetool config variables' '
+	remove_config_vars
+	git config merge.tool test-tool &&
+	git config mergetool.test-tool.cmd "cat \$LOCAL" &&
+
+	diff=$(git difftool --no-prompt branch) &&
+	test "$diff" = "branch" &&
+
+	# set merge.tool to something bogus, diff.tool to test-tool
+	git config merge.tool bogus-tool &&
+	git config diff.tool test-tool &&
+
+	diff=$(git difftool --no-prompt branch) &&
+	test "$diff" = "branch" &&
+
+	restore_test_defaults
+'
+
+test_done
-- 
1.6.2.1.303.g63699

Re: [PATCH] difftool: add various git-difftool tests

[Junio C Hamano]

David Aguilar <davvid@gmail.com> writes:

> +remove_config_vars()
> +{
> +	# Unset all config variables used by git-difftool
> +	git config --unset diff.tool
> +	git config --unset difftool.test-tool.cmd
> +	git config --unset merge.tool
> +	git config --unset mergetool.test-tool.cmd
> +	return 0
> +}
> +
> +restore_test_defaults()
> +{
> +	# Restores the test defaults used by several tests
> +	remove_config_vars
> +	unset GIT_DIFF_TOOL &&
> +	unset GIT_MERGE_TOOL &&
> +	unset GIT_DIFFTOOL_NO_PROMPT &&

I thought some shells' "unset" returns non-zero status when is given an
already unset variable.  I suspect you would want to drop the && chain
just like you did for remove_config_vars for the same reason.

> +	git config diff.tool test-tool &&
> +	git config difftool.test-tool.cmd "cat \$LOCAL"
> +}

'cat $LOCAL' would be much easier to read, wouldn't it?

> + ...
> +# Specify the diff tool using $GIT_DIFF_TOOL
> +test_expect_success 'GIT_DIFF_TOOL variable' '
> +	git config --unset diff.tool &&

You might want to lose && here in case the user told an earlier test that
sets the configuration to some value skipped (or such test failed), or
perhaps later somebody adds tests before this one that leaves the config
without this variable.

Other than that I didn't see major breakages.

Thanks.

Re: [PATCH] difftool: add various git-difftool tests

[David Aguilar]

On Tue, Mar 24, 2009 at 7:42 PM, Junio C Hamano <gitster@pobox.com> wrote:
> David Aguilar <davvid@gmail.com> writes:
>
>> +remove_config_vars()
>> +{
>> +     # Unset all config variables used by git-difftool
>> +     git config --unset diff.tool
>> +     git config --unset difftool.test-tool.cmd
>> +     git config --unset merge.tool
>> +     git config --unset mergetool.test-tool.cmd
>> +     return 0
>> +}
>> +
>> +restore_test_defaults()
>> +{
>> +     # Restores the test defaults used by several tests
>> +     remove_config_vars
>> +     unset GIT_DIFF_TOOL &&
>> +     unset GIT_MERGE_TOOL &&
>> +     unset GIT_DIFFTOOL_NO_PROMPT &&
>
> I thought some shells' "unset" returns non-zero status when is given an
> already unset variable.  I suspect you would want to drop the && chain
> just like you did for remove_config_vars for the same reason.


Ah, I learn something new everyday.  Thanks.
I've addressed these notes and am sending patch v2 now.



>
>> +     git config diff.tool test-tool &&
>> +     git config difftool.test-tool.cmd "cat \$LOCAL"
>> +}
>
> 'cat $LOCAL' would be much easier to read, wouldn't it?
>
>> + ...
>> +# Specify the diff tool using $GIT_DIFF_TOOL
>> +test_expect_success 'GIT_DIFF_TOOL variable' '
>> +     git config --unset diff.tool &&
>
> You might want to lose && here in case the user told an earlier test that
> sets the configuration to some value skipped (or such test failed), or
> perhaps later somebody adds tests before this one that leaves the config
> without this variable.
>
> Other than that I didn't see major breakages.
>
> Thanks.
>



-- 
    David