Re: [PATCH v7] diff: add config option relative

From: "Đoàn Trần Công Danh" <congdanhqx@gmail.com>
To: Laurent Arnoud <laurent@spkdev.net>
Cc: Junio C Hamano <gitster@pobox.com>,
	phillip.wood@dunelm.org.uk,
	"brian m. carlson" <sandals@crustytoothpaste.net>,
	git@vger.kernel.org
Subject: Re: [PATCH v7] diff: add config option relative
Date: Wed, 20 May 2020 06:01:24 +0700	[thread overview]
Message-ID: <20200519230124.GA12509@danh.dev> (raw)
In-Reply-To: <20200519193902.GA202573@spk-laptop>

On 2020-05-19 21:39:02+0200, Laurent Arnoud <laurent@spkdev.net> wrote:
> 
> +check_diff_relative_option () {
> +	dir=$1
> +	shift
> +	expect=$1
> +	shift
> +	relative_opt=$1
> +	shift
> +	test_expect_success "config diff.relative $relative_opt -p $*" "
> +		short_blob=$(git rev-parse --short $blob_file2) &&

We need to quote the first dollar, in order to run that command
inside the test.  Current version will run that git-rev-parse
outside of the test and interpolated into this command by outer shell.

	short_blob=\$(git rev-parse --short $blob_file2) &&

Sorry, I didn't think about this earlier.

> +	test_expect_success "config diff.relative $relative_opt -p $*" "
> +		short_blob_file1=$(git rev-parse --short $blob_file1) &&
> +		short_blob_file2=$(git rev-parse --short $blob_file2) &&

This test also needs to quote that dollar.
Beside that, LGTM.

-- 
Danh