Re: [PATCH v2 0/1] Make 'git commit' not accidentally lose staged content

[Junio C Hamano <gitster@pobox.com>]

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> This is about mixing "git add -p" and "git commit -a" (or "git commit
> <path>") where you may accidentally lose staged changes. After the
> discussion with Jonathan, I'm going with a bit different approach than
> v1, this behavior now becomes default, and if the user wants the old
> behavior back, they can use --clobber-index.
>
> Another change is "git commit <path>" is covered as well, as pointed
> out by Jacob.
>
> I will need to add some test cases of course, if this direction is
> still promising. One thing I'm not sure about is whether want to
> deliberately clobber the index often, then perhaps we should add a
> config key to bring the old behavior back.

It usually is safer (simply because you do not have to think about
it) to start a behaviour change like this as a strict opt-in to gain
confidence.

As I often see myself futzing with the same file, adding changes to
it incrementally, so that I can view progress in "diff --cached" and
"diff" output, it would be a serious usability regression if the
last step in the following sequence is rejected by default:

	edit X
	git add X
	git diff --cached X
	git diff X
	... repeat the above number of times ...
	git commit X ;# or "git commit -a" to finally conclude

On the other hand, if I am keeping some change that should never be
in a commit in the working tree file, and building the contents in
the index using "add -p" to incrementally, it would be the same
disaster as you are trying to prevent if I by mistake did a whole
path 'add', even if I catch myself doing so before running 'commit'
i.e.

	edit X
	git add -p X
	git diff --cached X
	git diff X
	... repeat the above number of times ...
	git add X ;# OOPS!
	git add . ;# OOPS! even worse!

Even though this does not involve "git commit -a" or "git commit X",
an unrecoverable damage that requires redoing the manual work is
already done.

The approach to check if the contents in the index matches that in
the HEAD per-path (i.e. "The contents we are adding to the index is
whole working tree contents for that path.  But the index already
has contents different from HEAD for the path---are we losing
information by doing this?"), is a very good one.  But for the
protection to be effective, I think "git commit" and "git add"
should be covered the same way, ideally with the same code and
possibly the same configuration knob and/or command line option to
control the behaviour.

If the information loss caused by the "add/commit X" or "add
-u/commit -a" is so serious that this new feature deserves to become
the default (which I do not yet think it is the case, by the way),
then we could even forbid "commit X" or "commit -a" when the paths
involved has difference between the index and the HEAD, without any
configuration knob or command line override for "commit", and then
tell the users to use "git add/rm" _with_ the override before coming
back to "git commit".

How should this new check intract with paths added with "add -N", by
the way?