Re: [PATCH] test ls-files with relative paths

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 1, 2011, 20:14 UTC
Message-ID: <7vhb60j3e1.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20110801180320.GA17556@toss>

Clemens Buchacher <drizzd@aon.at> writes:

Show 12 quoted lines

> Signed-off-by: Clemens Buchacher <drizzd@aon.at>
> ---
> On Sun, Jul 31, 2011 at 05:01:22PM -0700, Junio C Hamano wrote:
>> 
>> Sounds sane; tests?
>
> Certainly. I'm not testing partial commit since it uses the same
> codepath as ls-files --error-unmatch anyways.
>
>  t/t3005-ls-files-relative.sh |   74 ++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 74 insertions(+), 0 deletions(-)
>  create mode 100755 t/t3005-ls-files-relative.sh

Thanks.  This should be a part of the primary patch, not a standalone patch.

Show 11 quoted lines

> +test_expect_success 'prepare' '
> +	: >never-mind-me &&
> +	git add never-mind-me &&
> +	mkdir top &&
> +	(
> +		cd top &&
> +		mkdir sub &&
> +		x="x xa xbc xdef xghij xklmno" &&
> +		y=$(echo "$x" | tr x y) &&
> +		touch $x &&
> +		touch $y &&

These are not meant to be quoted; good.

Show 15 quoted lines

> +		cd sub &&
> +		git add ../x*
> +	)
> +'
> +
> +test_expect_success 'ls-files with mixed levels' '
> +	(
> +		cd top/sub &&
> +		f=$(cat <<-EOF
> +		../../never-mind-me
> +		../x
> +		EOF
> +		) &&
> +		echo "$f" >expect &&
> +		git ls-files $f >actual &&

This is a funny way to do this; why not cat the here-document into expect and feed $(cat expect) to ls-files? I suspect that it would become easier to read and certainly less confusing without need on the readers' side to scratch their heads wondering "does this $f need to be quoted?" and such.

Show 6 quoted lines

> +test_expect_success 'ls-files -c' '
> +	(
> +		cd top/sub &&
> +		for f in ../y*
> +		do
> +			echo "error: pathspec ${sq}${f}${sq} did not match any file(s) known to git."

Why not $sq$f$sq?  The above looks harder to read with the braces.

> +		set ../x* &&
> +		IFS="$new_line" && echo "$*" >>expect && unset IFS &&

That's a funny way to say "ls ../x*" isn't it?