Re: [PATCH 1/2] diff: turn --ita-invisible-in-index on by default

From

Jonathan Nieder <jrnieder@gmail.com>

Date

May 25, 2018, 16:43 UTC

Message-ID

<20180525164351.GA192362@aiede.svl.corp.google.com>

In-Reply-To

<20180525143042.GA10607@duynguyen.home>

Hi,

Duy Nguyen wrote:

>     $ echo haha > new; git add -N
>     $ git diff
>     diff --git a/new b/new
>     index e69de29..5ad28e2 100644
>     --- a/new
>     +++ b/new
>     @@ -0,0 +1 @@
>     +haha
>
> Notice that the diff does not tell you that 'new' is a new file. The
> diff with this patch gives you this
>
>     $ git diff
>     diff --git a/new b/new
>     new file mode 100644
>     index 0000000..5ad28e2
>     --- /dev/null
>     +++ b/new
>     @@ -0,0 +1 @@
>     +haha

[...]

> One consequence of this is you can't apply the diff generated with ita
> entries because the diff expects empty files to be already in the
> worktree. This to me does not make sense.
>
> Of course there's other things that also go along this line. Like if
> "git commit" does not add an ita entry, why should it appear in 'git
> diff --cached', which should show you what's to be committed.

Thankds for this context.  That helps a lot.

[...]

> Since this commit is already in 'next', it's too late to update the
> commit message now.

-- >8 -- 
Subject: Revert "diff: turn --ita-invisible-in-index on by default"

	echo hi >new-file
	git add -N new-file
	git diff HEAD

Before: shows addition of the new file
After: shows no change

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 builtin/diff.c          |  7 -------
 t/t2203-add-intent.sh   | 40 ++++++++--------------------------------
 t/t4011-diff-symlink.sh | 10 ++++------
 3 files changed, 12 insertions(+), 45 deletions(-)

diff --git a/builtin/diff.c b/builtin/diff.c
index b709b6e984..bfefff3a84 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -352,13 +352,6 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
 	rev.diffopt.flags.allow_external = 1;
 	rev.diffopt.flags.allow_textconv = 1;
 
-	/*
-	 * Default to intent-to-add entries invisible in the
-	 * index. This makes them show up as new files in diff-files
-	 * and not at all in diff-cached.
-	 */
-	rev.diffopt.ita_invisible_in_index = 1;
-
 	if (nongit)
 		die(_("Not a git repository"));
 	argc = setup_revisions(argc, argv, &rev, NULL);
diff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh
index 1d640a33f0..d9fb151d52 100755
--- a/t/t2203-add-intent.sh
+++ b/t/t2203-add-intent.sh
@@ -69,9 +69,9 @@ test_expect_success 'i-t-a entry is simply ignored' '
 	git add -N nitfol &&
 	git commit -m second &&
 	test $(git ls-tree HEAD -- nitfol | wc -l) = 0 &&
-	test $(git diff --name-only --ita-visible-in-index HEAD -- nitfol | wc -l) = 1 &&
-	test $(git diff --name-only HEAD -- nitfol | wc -l) = 0 &&
-	test $(git diff --name-only -- nitfol | wc -l) = 1
+	test $(git diff --name-only HEAD -- nitfol | wc -l) = 1 &&
+	test $(git diff --name-only --ita-invisible-in-index HEAD -- nitfol | wc -l) = 0 &&
+	test $(git diff --name-only --ita-invisible-in-index -- nitfol | wc -l) = 1
 '
 
 test_expect_success 'can commit with an unrelated i-t-a entry in index' '
@@ -99,13 +99,13 @@ test_expect_success 'cache-tree invalidates i-t-a paths' '
 
 	: >dir/bar &&
 	git add -N dir/bar &&
-	git diff --name-only >actual &&
+	git diff --cached --name-only >actual &&
 	echo dir/bar >expect &&
 	test_cmp expect actual &&
 
 	git write-tree >/dev/null &&
 
-	git diff --name-only >actual &&
+	git diff --cached --name-only >actual &&
 	echo dir/bar >expect &&
 	test_cmp expect actual
 '
@@ -186,19 +186,7 @@ test_expect_success 'rename detection finds the right names' '
 		cat >expected.3 <<-EOF &&
 		2 .R N... 100644 100644 100644 $hash $hash R100 third	first
 		EOF
-		test_cmp expected.3 actual.3 &&
-
-		git diff --stat >actual.4 &&
-		cat >expected.4 <<-EOF &&
-		 first => third | 0
-		 1 file changed, 0 insertions(+), 0 deletions(-)
-		EOF
-		test_cmp expected.4 actual.4 &&
-
-		git diff --cached --stat >actual.5 &&
-		: >expected.5 &&
-		test_cmp expected.5 actual.5
-
+		test_cmp expected.3 actual.3
 	)
 '
 
@@ -234,27 +222,15 @@ test_expect_success 'double rename detection in status' '
 	)
 '
 
-test_expect_success 'diff-files/diff-cached shows ita as new/not-new files' '
-	git reset --hard &&
-	echo new >new-ita &&
-	git add -N new-ita &&
-	git diff --summary >actual &&
-	echo " create mode 100644 new-ita" >expected &&
-	test_cmp expected actual &&
-	git diff --cached --summary >actual2 &&
-	: >expected2 &&
-	test_cmp expected2 actual2
-'
-
 test_expect_success 'apply --intent-to-add' '
 	git reset --hard &&
 	echo new >new-ita &&
 	git add -N new-ita &&
-	git diff >expected &&
+	git diff --ita-invisible-in-index >expected &&
 	grep "new file" expected &&
 	git reset --hard &&
 	git apply --intent-to-add expected &&
-	git diff >actual &&
+	git diff --ita-invisible-in-index >actual &&
 	test_cmp expected actual
 '
 
diff --git a/t/t4011-diff-symlink.sh b/t/t4011-diff-symlink.sh
index 108c012a3a..cf0f3a1ee7 100755
--- a/t/t4011-diff-symlink.sh
+++ b/t/t4011-diff-symlink.sh
@@ -139,13 +139,11 @@ test_expect_success SYMLINKS 'setup symlinks with attributes' '
 test_expect_success SYMLINKS 'symlinks do not respect userdiff config by path' '
 	cat >expect <<-\EOF &&
 	diff --git a/file.bin b/file.bin
-	new file mode 100644
-	index 0000000..d95f3ad
-	Binary files /dev/null and b/file.bin differ
+	index e69de29..d95f3ad 100644
+	Binary files a/file.bin and b/file.bin differ
 	diff --git a/link.bin b/link.bin
-	new file mode 120000
-	index 0000000..dce41ec
-	--- /dev/null
+	index e69de29..dce41ec 120000
+	--- a/link.bin
 	+++ b/link.bin
 	@@ -0,0 +1 @@
 	+file.bin

-- 
2.17.0.921.gf22659ad46