Thread (12 messages) 12 messages, 3 authors, 2018-05-27

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

From: Duy Nguyen <hidden>
Date: 2018-05-25 16:59:35

On Fri, May 25, 2018 at 6:43 PM, Jonathan Nieder [off-list ref] wrote:
I think it should be reverted from 'next' because of the unintended
change to the behavior of "git diff HEAD".
Ah. That is indeed unintended. I still don't know how this change
affects that (but that's probably why it slipped through in the first
place). While there, perhaps you could add a test in t2203 just to
make sure I will not break it again in the next attempt?

I agree that it should be reverted. Or if it's possible to eject it
from next, then it's even better since I will need to fix up the
second patch in that series anyway. Junio?
quoted hunk ↗ jump to hunk
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
-- 
Duy
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help