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

From: Duy Nguyen <pclouds@gmail.com>
Date: Oct 7, 2016, 12:56 UTC
Message-ID: <CACsJy8DiGoaKZZ1je=3L3y4odVHB7wLvvKs9pccjiN=-UeqeVw@mail.gmail.com>
In-Reply-To: <xmqqwphljnlj.fsf@gitster.mtv.corp.google.com>

On Fri, Oct 7, 2016 at 2:15 AM, Junio C Hamano <gitster@pobox.com> wrote:

Show 17 quoted lines

> Duy Nguyen <pclouds@gmail.com> writes:
>
>> On Tue, Oct 4, 2016 at 11:15 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>> Duy Nguyen <pclouds@gmail.com> 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.

No problem. I do appreciate a straight shoot down though. Many of my topics have been going on for months (ones not in 'pu') and seeing it rejected near the end is worse than stopping working on them early.

Show 6 quoted lines

> 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 don't know if we have had any external diff calls in our shell-based commands. I don't read them often. Regardless, people do use "git diff" and it should show the "right thing" (I know it's subjective). Or at least be consistent with both git-commit and git-status.

Show 5 quoted lines

> [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

Off topic. This reminds me of an old patch about apply and ita [1] but that one is not the same here

Show 6 quoted lines

> 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".

We could detect "no repo available" and ignore the index, I guess.

Show 10 quoted lines

> "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.

--cached tells you to put new contents in the index. I-ta entries, being a reminder to add stuff, don't really fit in here because you want to add contents _now_, i think. After a successful "git apply --cached", a "git commit" should contain exactly what the applied patch has. If new files are i-t-a entries instead, then the new commit would not be the same as the patch.

[1] https://public-inbox.org/git/1451181092-26054-4-git-send-email-pclouds@gmail.com/

-- 
Duy