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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Duy Nguyen <pclouds@xxxxxxxxx> writes:

> On Tue, Oct 4, 2016 at 11:15 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
>> Duy Nguyen <pclouds@xxxxxxxxx> writes:
>>
>>> We don't use it internally _yet_. I need to go through all the
>>> external diff code and see --shift-ita should be there. The end goal
>>> is still changing the default behavior and getting rid of --shift-ita,
>>
>> I do not agree with that endgame, and quite honestly I do not want
>> to waste time reviewing such a series.

I definitely shouldn't have said that, especially "waste".  Many
issues around i-t-a and diff make my head hurt when I think about
them [*1*], but not wanting to spend time that gets my
head hurt and not wanting to waste time are totally different
things.  My apologies.

I missed something curious in your statement above, i.e. "external
diff".  I thought we have pretty much got rid of all the invocation
of "git diff" via the run_command() interface and we do not need the
command line option (we only need the options->shift_ita so that
callers like "git status" can seletively ask for it when making
internal calls), and that is why I didn't want to see it.

> I do not believe current "diff" behavior wrt. i-t-a entries is right
> either. There's no point in pursuing this series then. Feel free to
> revert 3f6d56d (commit: ignore intent-to-add entries instead of
> refusing - 2012-02-07) and bring back the old i-t-a behavior. All the
> problems would go away.

It is way too late to revert 3f6d56d.  Even though I think it was
overall a mistake to treat i-t-a as "not exist" while committing,
people are now used to the behaviour with that change, and we need
to make our progress within the constraint of the real world.


[Footnote]

Here is one of the things around i-t-a and diff.  If you make "git
diff" (between the index and the working tree) report "new" file, it
would imply that "git apply" run without "--index" should create an
ita entry in the index for symmetry, wouldn't it?  That by itself
can be seen as an improvement (we no longer would have to say that
"git apply patchfile && git commit -a" that is run in a clean state
will forget new files the patchfile creates), but it also means we
now need a repository in order to run "git apply" (without "--index"),
which is a problem, as "git apply" is often used as a better "patch".

"git apply --cached" may also become "interesting".  A patch that
would apply cleanly to HEAD should apply cleanly if you did this:

    $ git read-tree HEAD
    $ git apply --cached <patch

no matter what the working tree state is.  Should a patch that
creates a "new" file add contents to the index, or just an i-t-a
entry?  I could argue it both ways, but either is quite satisfactory
and makes my head hurt.



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]