git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Duy Nguyen <pclouds@gmail.com>
Date
May 25, 2018, 16:59 UTC
Message-ID
<CACsJy8Bit2QcVMTnKfSuUjXbxnjHMLLd5LjV36zbnQyc=7c8Wg@mail.gmail.com>
In-Reply-To
<20180525164351.GA192362@aiede.svl.corp.google.com>
On Fri, May 25, 2018 at 6:43 PM, Jonathan Nieder <jrnieder@gmail.com> 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?

Show 157 quoted lines
> 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 <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
-- 
Duy
Previous: Jonathan NiederNext: Nguyễn Thái Ngọc Duy
Message 9 of 15 in “diff: turn --ita-invisible-in-index on by default”
  1. 1/2 diff: turn --ita-invisible-in-index on by defaultNguyễn Thái Ngọc Duy, May 13, 2018
  2. 2/2 apply: add --intent-to-addNguyễn Thái Ngọc Duy, May 13, 2018
  3. Junio C HamanoMay 14, 2018
  4. Duy NguyenMay 20, 2018
  5. Junio C HamanoMay 21, 2018
  6. Jonathan NiederMay 25, 2018
  7. Duy NguyenMay 25, 2018
  8. Jonathan NiederMay 25, 2018
  9. Duy NguyenMay 25, 2018
  10. 0/4 Fix i-t-a entries in git-diff and git-applyNguyễn Thái Ngọc Duy, May 26, 2018
  11. 2/4 diff: turn --ita-invisible-in-index on by defaultNguyễn Thái Ngọc Duy, May 26, 2018
  12. 1/4 diff: ignore --ita-[in]visible-in-index when diffing worktree-to-treeNguyễn Thái Ngọc Duy, May 26, 2018
  13. Eric SunshineMay 27, 2018
  14. 3/4 t2203: add a test about "diff HEAD" caseNguyễn Thái Ngọc Duy, May 26, 2018
  15. 4/4 apply: add --intent-to-addNguyễn Thái Ngọc Duy, May 26, 2018

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.