All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Duy Nguyen <pclouds@gmail.com>
Cc: Git Mailing List <git@vger.kernel.org>
Subject: Re: [PATCH 1/3] Resurrect "diff-lib.c: adjust position of i-t-a entries in diff"
Date: Thu, 06 Oct 2016 12:15:20 -0700	[thread overview]
Message-ID: <xmqqwphljnlj.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <CACsJy8D7c8Z_ugasn_scf391+C6GxJp1CYwHY4ndvVtLiJzxnQ@mail.gmail.com> (Duy Nguyen's message of "Wed, 5 Oct 2016 16:43:13 +0700")

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.

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.

  reply	other threads:[~2016-10-06 19:15 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-09-28 11:43 [PATCH 0/3] i-t-a entries in git-status, and git-commit Nguyễn Thái Ngọc Duy
2016-09-28 11:43 ` [PATCH 1/3] Resurrect "diff-lib.c: adjust position of i-t-a entries in diff" Nguyễn Thái Ngọc Duy
2016-09-28 19:28   ` Junio C Hamano
2016-09-28 20:33     ` Junio C Hamano
2016-10-03 10:36     ` Duy Nguyen
2016-10-04 16:15       ` Junio C Hamano
2016-10-05  9:43         ` Duy Nguyen
2016-10-06 19:15           ` Junio C Hamano [this message]
2016-10-07 12:56             ` Duy Nguyen
2016-10-10 23:08               ` Junio C Hamano
2016-09-28 11:43 ` [PATCH 2/3] diff-lib.c: enable --shift-ita in index_differs_from() Nguyễn Thái Ngọc Duy
2016-09-28 18:49   ` Junio C Hamano
2016-09-28 11:43 ` [PATCH 3/3] commit: don't be fooled by ita entries when creating initial commit Nguyễn Thái Ngọc Duy
2016-09-28 11:51 ` [PATCH 0/3] i-t-a entries in git-status, and git-commit Duy Nguyen
2016-10-24 10:42 ` [PATCH 0/4] nd/ita-empty-commit update Nguyễn Thái Ngọc Duy
2016-10-24 10:42   ` [PATCH 1/4] diff-lib: allow ita entries treated as "not yet exist in index" Nguyễn Thái Ngọc Duy
2016-10-24 10:42   ` [PATCH 2/4] diff: add --ita-[in]visible-in-index Nguyễn Thái Ngọc Duy
2016-10-24 10:42   ` [PATCH 3/4] commit: fix empty commit creation when there's no changes but ita entries Nguyễn Thái Ngọc Duy
2016-10-24 10:42   ` [PATCH 4/4] commit: don't be fooled by ita entries when creating initial commit Nguyễn Thái Ngọc Duy
2016-10-24 17:58   ` [PATCH 0/4] nd/ita-empty-commit update Junio C Hamano
2016-10-25  9:34     ` Duy Nguyen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqwphljnlj.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=pclouds@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.