threads / patch / 18481

patchdifftool: add git-difftool to the list of commands

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

## tl;dr

4 messages between Mar 23, 2009 and Mar 25, 2009. Diffs are folded; open one to read it.

replies: 3people: 2as markdown or json

David Aguilar· Mar 23, 2009, 08:41 UTC · lore

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

Show changes to command-list.txt +1 −0

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

David Aguilar· Mar 23, 2009, 08:41 UTC · re: David Aguilar · lore

[PATCH] difftool: add various git-difftool tests

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

Show changes to t/t7800-difftool.sh +139 −0

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

Junio C Hamano· Mar 25, 2009, 02:42 UTC · re: David Aguilar · lore

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

David Aguilar <davvid@gmail.com> writes:

Show 17 quoted lines

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

David Aguilar· Mar 25, 2009, 03:57 UTC · re: Junio C Hamano · lore

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

On Tue, Mar 24, 2009 at 7:42 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 23 quoted lines

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

Show 21 quoted lines

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