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

[Junio C Hamano <gitster@pobox.com>]

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.