Re: [PATCH] diff-files: treat "i-t-a" files as "not-in-index"

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 18, 2020, 22:33 UTC
Message-ID: <xmqqk104knrs.fsf@gitster.c.googlers.com>
In-Reply-To: <20200611161640.52156-1-shrinidhi.kaushik@gmail.com>

Srinidhi Kaushik <shrinidhi.kaushik@gmail.com> writes:

Show 5 quoted lines

> -test_expect_success 'diff-files/diff-cached shows ita as new/not-new files' '
> +test_expect_success 'diff/diff-cached shows ita as new/not-new files' '
>  	git reset --hard &&
>  	echo new >new-ita &&
>  	git add -N new-ita &&

Interesting.

I thought that the test originally tested "diff-files" and "diff-index --cached" and a modernization patch forgot to update the title when the test body was changed to use "diff" and "diff --cached", but that is not the case here. When 0231ae71 (diff: turn --ita-invisible-in-index on by default, 2018-05-26) added this test, it gave a wrong title from the beginning.

Nice catch.

Show 7 quoted lines

> @@ -243,6 +243,29 @@ test_expect_success 'diff-files/diff-cached shows ita as new/not-new files' '
>  	test_must_be_empty actual2
>  '
>  
> +test_expect_success 'diff-files shows i-t-a files as new files' '
> +	git reset --hard &&
> +	touch empty &&

Use of "touch" gives a wrong impression that you care about the file timestamp; use something like ": >empty &&" instead when you care about the presence of the file and do not care about its timestamp.

> +	content="foo" &&
> +	echo $content >not-empty &&

The quoting decision is backwards in these two lines. It is OK not to quote when the right hand side literal is clearly a single word without $IFS. On the other hand, it is a good practice to always quote when using what is in a "$variable".

Show 18 quoted lines

> +	git add -N empty not-empty &&
> +	git diff-files -p >actual &&
> +	hash_e=$(git hash-object empty) &&
> +	hash_n=$(git hash-object not-empty) &&
> +	cat >expect <<-EOF &&
> +	diff --git a/empty b/empty
> +	new file mode 100644
> +	index 0000000..$(git rev-parse --short $hash_e)
> +	diff --git a/not-empty b/not-empty
> +	new file mode 100644
> +	index 0000000..$(git rev-parse --short $hash_n)
> +	--- /dev/null
> +	+++ b/not-empty
> +	@@ -0,0 +1 @@
> +	+$content
> +	EOF
> +	test_cmp expect actual
> +'

OK. Do we want to show what happens when "diff" and "diff --cached" are run with these two "added but not quite added yet" paths to contrast with this new case?

Thanks.