Re: [PATCH 2/4] Resurrect "diff-lib.c: adjust position of i-t-a entries in diff"

From: Duy Nguyen <pclouds@gmail.com>
Date: Jun 7, 2016, 12:04 UTC
Message-ID: <CACsJy8DO+KU59LbQ0ZtTax6DZKqYmZVT6MHLmNK73GkO9qHf_w@mail.gmail.com>
In-Reply-To: <xmqqtwh6uk0k.fsf@gitster.mtv.corp.google.com>

On Tue, Jun 7, 2016 at 3:42 AM, Junio C Hamano <gitster@pobox.com> wrote:

Show 9 quoted lines

> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
>
>> +--shift-ita::
>> +     By default entries added by "git add -N" appear as an existing
>> +     empty file in "git diff" and a new file in "git diff --cached".
>> +     This option makes the entry appear as a new file in "git diff"
>> +     and non-existent in "git diff --cached".
>
> I do not think this should exist at the UI level,

I need it. I do "git diff --stat" and "git diff --cached --stat" a lot more often than "git status". Without this option, I'm stuck with the old behavior.

Show 8 quoted lines

> even though the
> use of it in wt-status.c (below) makes a very good sense at least
> as a temporary band-aid.
>
> At the philosophical level, I however think this "I-T-A does not
> logically exist in the index (yet)" is a mistake, and "an option
> controls if I-T-A does or does not exist depending on who calls it"
> is even worse; it is a road to insanity.

i-t-a entries have dual personality (perhaps because it's implemented as an index entry). Although I think the "does not exist" aspect will win in most cases. The intention behind the revert is we have more time to examine case by case and gradually convert them. Maybe in the end one behavior wins and we no longer need another. A thought of keeping i-t-a entries in an index extension instead crossed my mind. It may simplify things a bit (e.g. there's no "ghost" entries any more and active_nr in 3/4 can remain "the number of _real_ entries"). The few parts that do need to know about i-t-a entries need explict modification (probably git-reset and git-diff). But I don't know yet if it would just lead to another nightmare.

Show 8 quoted lines

> For example, because I-T-A does not logically exist in the index,
> "git reset --hard" should not remove it but make it untracked again
> (but I do not think it does). After "git add -N foo", because "foo"
> does not exist in the index, "git clean" should remove it for the
> definition of what's in the index to be logically consistent, but
> the whole intent of "add -N" is that the user meant it is worth
> checking into sometime in the future, which contradicts with its
> removal upon "clean".

I think we should fix them. I started that and so far only 4d55200 (grep: make it clear i-t-a entries are ignored - 2015-12-27) has made it to 'master'.

> So, I dunno.

I just remembered why the old behavior (abort to commit if i-t-a entries are present) bugged me: it does not work well with splitting changes in worktree into multiple commits (e.g. with "git add -p"). Even though I want git remind me to commit an i-t-a entry in the end, it does not necessarily mean I have to do it in the next commit, which may cover a bunch of files except that i-t-a file. I don't see any way around that except ignoring i-t-a entries at commit time. If there's another way, I'm all ears.

-- 
Duy