Aborting "git commit --interactive" discards updates to index (was: Re: [ANNOUNCE] Git 1.7.6)

Aborting "git commit --interactive" discards updates to index (was: Re: [ANNOUNCE] Git 1.7.6)

[demerphq]

On 27 June 2011 17:59, Junio C Hamano <gitster@pobox.com> wrote:
> The latest feature release Git 1.7.6 is available at the usual
> places:
>
>  http://www.kernel.org/pub/software/scm/git/
[snip]
>  * Aborting "git commit --interactive" discards updates to the index
>   made during the interactive session.

Hi, I am wondering why this change was made?

I can sort of understand if people do CTL-C during an interactive
commit that throwing the results away might be useful (although I
don't see why personally), but what I don't understand at all is why
it happens when the "add --interactive" is completed properly, but the
user decided not to actually commit. For me and a number of colleagues
the normal reason we exit the commit part (that is exit the editor
without modifying the commit message) is because we realize we forgot
something, such as adding a new file, and want to exit out and re-add
it. I am writing this after spending about 45 minutes showing a
colleague how to use git commit --interactive, when we realized that
we had forgotten to add a file. Needless to say he wasn't too happy
about losing 45 minutes work and having to redo it.

The new behavior potentially means that a lot of work (such as via the
'e' option) is instantly discarded. I don't understand why this is
perceived to be sensible behavior -- I thought the default policy for
git would be to not lose work!

I would really like an git config option to revert to the previous
behavior of not throwing away what I staged, or even better have git
commit --interactive ask me what I want to do, after all, it is an
interactive process so it seems reasonable it asks before it does
something like throw away work.

Yves

-- 
perl -Mre=debug -e "/just|another|perl|hacker/"

Re: Aborting "git commit --interactive" discards updates to index

[Junio C Hamano]

demerphq <demerphq@gmail.com> writes:

> On 27 June 2011 17:59, Junio C Hamano <gitster@pobox.com> wrote:
>> The latest feature release Git 1.7.6 is available at the usual
>> places:
>>
>>  http://www.kernel.org/pub/software/scm/git/
> [snip]
>>  * Aborting "git commit --interactive" discards updates to the index
>>   made during the interactive session.
>
> Hi, I am wondering why this change was made?

I wasn't directly involved in this particular part of the design of what
should happen to the index when a commit is aborted, so I would be a bad
person to give you the first answer, but let's try.

If a "commit" session is aborted, it is logical to revert whatever has
been done inside that session as a single logical unit, so I do not
particularly find the current behaviour so confusing. It might even make
more sense if we update "commit -i" and "commit -a" to also revert the
index modification when the command is aborted for consistency.

You are welcome to rehash the age old discussion, though. Personally I do
not care very deeply either way. I would never use "commit --interactive"
myself, and I would not encourage others to use it, either, even if we do
not worry about the behaviour when a commit is aborted.

One thread of interest (there are others, as this change was rerolled at
least a few times) may be

    http://thread.gmane.org/gmane.comp.version-control.git/173033/focus=173035

Having said all that,...

> .... I am writing this after spending about 45 minutes showing a
> colleague how to use git commit --interactive, when we realized that
> we had forgotten to add a file....

... if your partial commit is so complex that you need to spend 45 minutes
to sift what to be commited and what to be left out, you are much better
off to run "git add -i" to prepare the index, "git stash save -k" to check
out what is to be committed (and stash away what are to be left out) so
that you can make sure what you are committing is what you thought are
committing (by asking "git diff" and "make test" for example), and after
convincing yourself that you made a good state in the index, make a commit
with "git commit" (without any other arguments) and conclude it with "git
stash pop" to recover the changes that you decided to leave out.

"commit --interactive" robs me from that crucial "verification" step, and
that is why I wouldn't be a user or an advocate of this "misfeature".

By the way, why did you draw Ævar into this discussion? I do not think he
was involved in any way with the design or implementation of this command.

Re: Aborting "git commit --interactive" discards updates to index

[demerphq]

On 7 January 2012 06:08, Junio C Hamano <gitster@pobox.com> wrote:
> demerphq <demerphq@gmail.com> writes:
>
>> On 27 June 2011 17:59, Junio C Hamano <gitster@pobox.com> wrote:
>>> The latest feature release Git 1.7.6 is available at the usual
>>> places:
>>>
>>>  http://www.kernel.org/pub/software/scm/git/
>> [snip]
>>>  * Aborting "git commit --interactive" discards updates to the index
>>>   made during the interactive session.
>>
>> Hi, I am wondering why this change was made?
>
> I wasn't directly involved in this particular part of the design of what
> should happen to the index when a commit is aborted, so I would be a bad
> person to give you the first answer, but let's try.
>
> If a "commit" session is aborted, it is logical to revert whatever has
> been done inside that session as a single logical unit, so I do not
> particularly find the current behaviour so confusing. It might even make
> more sense if we update "commit -i" and "commit -a" to also revert the
> index modification when the command is aborted for consistency.
>
> You are welcome to rehash the age old discussion, though. Personally I do
> not care very deeply either way. I would never use "commit --interactive"
> myself, and I would not encourage others to use it, either, even if we do
> not worry about the behaviour when a commit is aborted.

If I were to provide a patch to make this behavior configurable would
you have any objections? I personally much prefer the old behaviour. I
want it to leave the stuff in my index.

>
> One thread of interest (there are others, as this change was rerolled at
> least a few times) may be
>
>    http://thread.gmane.org/gmane.comp.version-control.git/173033/focus=173035

Thanks.

> Having said all that,...
>
>> .... I am writing this after spending about 45 minutes showing a
>> colleague how to use git commit --interactive, when we realized that
>> we had forgotten to add a file....
>
> ... if your partial commit is so complex that you need to spend 45 minutes
> to sift what to be commited and what to be left out, you are much better

I was showing a colleague how to use the features it provides on a
large patch where the committer wanted to keep various bits and not
keep others.

> off to run "git add -i" to prepare the index, "git stash save -k" to check
> out what is to be committed (and stash away what are to be left out) so
> that you can make sure what you are committing is what you thought are
> committing (by asking "git diff" and "make test" for example), and after

Isnt this what the diff option in commit interactive is for?
Personally I tend to review patches in detail before I push them, not
necessarily before I commit them.

> convincing yourself that you made a good state in the index, make a commit
> with "git commit" (without any other arguments) and conclude it with "git
> stash pop" to recover the changes that you decided to leave out.

I personally have never had an issue with git commit --interactive so
this procedure seems really convoluted to me, and a lot harder to
teach to a colleague than "use git commit --interactive". Is there a
real problem it solves?

> "commit --interactive" robs me from that crucial "verification" step, and
> that is why I wouldn't be a user or an advocate of this "misfeature".

I understand. But that is a workflow directed to statically testable
library code. Our workflow doesn't really depend on a "make test"
phase. Also, if I calling git commit --interactive (or git add -i),
then I am as confident my code works as I can be, and the reason I am
doing an interactive step is for instance to edit out debug lines, or
separate out whitespace changes from code changes, or to break my
change set into several logical groups.

> By the way, why did you draw Ævar into this discussion? I do not think he
> was involved in any way with the design or implementation of this command.

He is a git hacker, and is a friend and colleague. We both work for a
largish dotcom which uses git as our primary version control and we
have collaborated on a tool I wrote to use git to manage our rollout
processes. So when something git comes up it is natural to me to CC
him.

cheers,
Yves

-- 
perl -Mre=debug -e "/just|another|perl|hacker/"

Re: Aborting "git commit --interactive" discards updates to index

[Junio C Hamano]

demerphq <demerphq@gmail.com> writes:

> On 7 January 2012 06:08, Junio C Hamano <gitster@pobox.com> wrote:
> ...
>> You are welcome to rehash the age old discussion, though. Personally I do
>> not care very deeply either way. I would never use "commit --interactive"
>> myself, and I would not encourage others to use it, either, even if we do
>> not worry about the behaviour when a commit is aborted.
>
> If I were to provide a patch to make this behavior configurable would
> you have any objections?

You are welcome to rehash the discussion.

I am not a dictator and will listen to other people on the list for their
opinions, and I cannot say if such a patch will be accepted or not without
seeing how well it is done.

>> ... off to run "git add -i" to prepare the index, "git stash save -k" to check
>> out what is to be committed (and stash away what are to be left out) so
>> that you can make sure what you are committing is what you thought are
>> committing (by asking "git diff" and "make test" for example), and after
>
> Isnt this what the diff option in commit interactive is for?

Not at all.

That is to help the user incrementally in the process and not a
replacement for the final eyeballing of the result.

Neither the patch shown in "commit -v", whose primary purpose is to aid
the user to write a better log message.