Re: [PATCH 1/2] diff: turn --ita-invisible-in-index on by default
From: Jonathan Nieder <hidden>
Date: 2018-05-25 16:44:03
Subsystem:
the rest · Maintainer:
Linus Torvalds
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.
I think it should be reverted from 'next' because of the unintended change to the behavior of "git diff HEAD". Here is what I have applied internally. -- >8 -- Subject: Revert "diff: turn --ita-invisible-in-index on by default" This reverts commit 992118dd95b052bc82a795fd3a8978bea0fe5c0e. That change is promising, since it improves the behavior of "git diff" and "git diff --cached" by correctly showing an intent-to-add entry as no file. Unfortunately, though, it breaks "git diff HEAD" in the process: echo hi >new-file git add -N new-file git diff HEAD Before: shows addition of the new file After: shows no change Noticed because the new --ita-invisible-in-index default broke a script using "git diff --name-only --diff-filter=A master" to get a list of added files. Arguably that script should use diff-index instead, which would have sidestepped the issue, but the new behavior is unexpected in interactive use as well. Signed-off-by: Jonathan Nieder <redacted> --- 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