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

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

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

> -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.

> @@ -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".

> +	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.