Re: [PATCH 2/3] difftool: avoid $GIT_DIR and $GIT_WORK_TREE

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

David Aguilar <davvid@gmail.com> writes:

> Environment variables are global and hard to reason about.
> Use the `--git-dir` and `--work-tree` arguments when invoking `git`
> instead of relying on the environment.
>
> Add a test to ensure that difftool's dir-diff feature works when these
> variables are present in the environment.

Yes, that is a very good idea.  I was wondering what would happen
with that "must-not-unset-nor-smudge if it already is set" thing.

> diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
> index 42a2929..fa43c24 100755
> --- a/t/t7800-difftool.sh
> +++ b/t/t7800-difftool.sh
> @@ -412,6 +412,22 @@ run_dir_diff_test 'difftool --dir-diff from subdirectory' '
>  	)
>  '
>  
> +run_dir_diff_test 'difftool --dir-diff from subdirectory with GIT_DIR set' '
> +	(
> +		GIT_DIR=$(pwd)/.git &&
> +		export GIT_DIR &&
> +		GIT_WORK_TREE=$(pwd) &&
> +		export GIT_WORK_TREE &&
> +		cd sub &&
> +		git difftool --dir-diff $symlinks --extcmd ls \
> +			branch -- sub >output &&
> +		sane_unset GIT_WORK_TREE &&
> +		sane_unset GIT_DIR &&

It is not wrong per-se, but as you are in a subshell, you do not
have to unset these, I would think.  Not worth a reroll, but unless
I am overlooking something (in which case please holler) I'm
inclined to remove these two lines myself while queuing the series.

Thanks.

> +		grep sub output &&
> +		! grep file output
> +	)
> +'