threads / bug / 22494

[BUG] - "git commit --amend" commits, when exiting the editor with no changes written

Subject: [BUG] - "git commit --amend" commits, when exiting the editor with no changes written

## tl;dr

45 messages between Feb 2, 2010 and Feb 3, 2010.

replies: 44people: 13as markdown or json

Eugene Sajine· Feb 2, 2010, 20:07 UTC · lore
Hi,
It seems to me it is a bug:

When git commit --amend is executed it fires up vi so i can change the commit message. If I have something staged, then when I'm exiting vi using :q (without doing/writing any changes), it still commits everything staged with old message.

I believe it should actually abort amending and return to the state before the "git commit --amend" was issued.

Thanks, Eugene

Jacob Helwig· Feb 2, 2010, 20:14 UTC · re: Eugene Sajine · lore

Re: [BUG] - "git commit --amend" commits, when exiting the editor with no changes written

On Tue, Feb 2, 2010 at 12:07, Eugene Sajine <euguess@gmail.com> wrote:
Show 15 quoted lines
> Hi,
>
> It seems to me it is a bug:
>
> When git commit --amend is executed it fires up vi so i can change the
> commit message. If I have something staged, then when I'm exiting vi
> using :q (without doing/writing any changes), it still commits
> everything staged with old message.
>
> I believe it should actually abort amending and return to the state
> before the "git commit --amend" was issued.
>
> Thanks,
> Eugene
>

I don't think this is actually the right way to go. A _very_ common use case for "commit --amend" is to add things to the previous commit, without changing the commit message at all. Yes, you can avoid having to fire up the editor at all in this case, but it's still a perfectly valid thing to want to do.

If you want to abort amending, and return to the state before the "git commit --amend", you can remove all non-comment lines (IE: write out an "empty" commit message), or have your editor exit with a non-zero exit code (Use ":cquit").

The comment block that comes up in your editor mentions this first method, explicitly. "..., and an empty message aborts the commit."

-Jacob
Avery Pennarun· Feb 2, 2010, 20:27 UTC · re: Jacob Helwig · lore

Re: [BUG] - "git commit --amend" commits, when exiting the editor with no changes written

On Tue, Feb 2, 2010 at 3:14 PM, Jacob Helwig <jacob.helwig@gmail.com> wrote:
Show 14 quoted lines
> On Tue, Feb 2, 2010 at 12:07, Eugene Sajine <euguess@gmail.com> wrote:
>> When git commit --amend is executed it fires up vi so i can change the
>> commit message. If I have something staged, then when I'm exiting vi
>> using :q (without doing/writing any changes), it still commits
>> everything staged with old message.
>>
>> I believe it should actually abort amending and return to the state
>> before the "git commit --amend" was issued.
>
> I don't think this is actually the right way to go.  A _very_ common
> use case for "commit --amend" is to add things to the previous commit,
> without changing the commit message at all.  Yes, you can avoid having
> to fire up the editor at all in this case, but it's still a perfectly
> valid thing to want to do.

I agree and I do this all the time. However, I've also done it accidentally before I learned the "remove the commit message and save" trick.

Perhaps what's really missing is more documentation of how to "unamend" if you change your mind :) I happen to know about "git reset HEAD@{1}" but it's not terribly obvious.

Avery
Eugene Sajine· Feb 2, 2010, 20:47 UTC · re: Avery Pennarun · lore

Re: [BUG] - "git commit --amend" commits, when exiting the editor with no changes written

On Tue, Feb 2, 2010 at 3:27 PM, Avery Pennarun <apenwarr@gmail.com> wrote:
Show 26 quoted lines
> On Tue, Feb 2, 2010 at 3:14 PM, Jacob Helwig <jacob.helwig@gmail.com> wrote:
>> On Tue, Feb 2, 2010 at 12:07, Eugene Sajine <euguess@gmail.com> wrote:
>>> When git commit --amend is executed it fires up vi so i can change the
>>> commit message. If I have something staged, then when I'm exiting vi
>>> using :q (without doing/writing any changes), it still commits
>>> everything staged with old message.
>>>
>>> I believe it should actually abort amending and return to the state
>>> before the "git commit --amend" was issued.
>>
>> I don't think this is actually the right way to go.  A _very_ common
>> use case for "commit --amend" is to add things to the previous commit,
>> without changing the commit message at all.  Yes, you can avoid having
>> to fire up the editor at all in this case, but it's still a perfectly
>> valid thing to want to do.
>
> I agree and I do this all the time. However, I've also done it
> accidentally before I learned the "remove the commit message and save"
> trick.
>
> Perhaps what's really missing is more documentation of how to
> "unamend" if you change your mind :)  I happen to know about "git
> reset HEAD@{1}" but it's not terribly obvious.
>
> Avery
>

Well, i understand this logic but it seems to me that the issue is a little bit deeper than that. I just realized that the "rebase -i" is doing the same thing.

If you're just exiting the vi using :q without writing changes it will perform rebase! I'm 100% sure it is not OK.

So, it seems that it will do that every time it has some *prepopulated* message in the editor, so it doesn't understand when changes are written and when they are not.

I believe all those actions should be treated exactly the same way as the commit itself. If I'm writing/saving changes - it is good to go. If not - abort. I.e. the message can be prepopulated in the editor, but it should be the same state as when you create a new message for commit, i.e. git should not know anything about that message.

Thanks, Eugene

Wincent Colaiuta· Feb 2, 2010, 20:58 UTC · re: Eugene Sajine · lore

Re: [BUG] - "git commit --amend" commits, when exiting the editor with no changes written

El 02/02/2010, a las 21:47, Eugene Sajine escribió:
Show 43 quoted lines
> On Tue, Feb 2, 2010 at 3:27 PM, Avery Pennarun <apenwarr@gmail.com>  
> wrote:
>> On Tue, Feb 2, 2010 at 3:14 PM, Jacob Helwig  
>> <jacob.helwig@gmail.com> wrote:
>>> On Tue, Feb 2, 2010 at 12:07, Eugene Sajine <euguess@gmail.com>  
>>> wrote:
>>>> When git commit --amend is executed it fires up vi so i can  
>>>> change the
>>>> commit message. If I have something staged, then when I'm exiting  
>>>> vi
>>>> using :q (without doing/writing any changes), it still commits
>>>> everything staged with old message.
>>>>
>>>> I believe it should actually abort amending and return to the state
>>>> before the "git commit --amend" was issued.
>>>
>>> I don't think this is actually the right way to go.  A _very_ common
>>> use case for "commit --amend" is to add things to the previous  
>>> commit,
>>> without changing the commit message at all.  Yes, you can avoid  
>>> having
>>> to fire up the editor at all in this case, but it's still a  
>>> perfectly
>>> valid thing to want to do.
>>
>> I agree and I do this all the time. However, I've also done it
>> accidentally before I learned the "remove the commit message and  
>> save"
>> trick.
>>
>> Perhaps what's really missing is more documentation of how to
>> "unamend" if you change your mind :)  I happen to know about "git
>> reset HEAD@{1}" but it's not terribly obvious.
>>
>> Avery
>>
>
> Well, i understand this logic but it seems to me that the issue is a
> little bit deeper than that.
> I just realized that the "rebase -i" is doing the same thing.
>
> If you're just exiting the vi using :q without writing changes it will
> perform rebase! I'm 100% sure it is not OK.

As has already been pointed out, an extremely common workflow for "git commit --amend" is to include additional changes that were accidentally overlooked in the initial commit, and no changes are made to the commit message.

Changing the behavior would break that perfectly reasonable, widely- practised workflow, and _that_ would be 100% not ok.

Lucky for you, you've already been provided with two means of aborting (empty commit message, and non-zero editor exit), so you might as well start getting used to them.

Wincent
Eugene Sajine· Feb 2, 2010, 21:56 UTC · re: Wincent Colaiuta · lore

Re: [BUG] - "git commit --amend" commits, when exiting the editor with no changes written

Show 8 quoted lines
>
> As has already been pointed out, an extremely common workflow for "git
> commit --amend" is to include additional changes that were accidentally
> overlooked  in the initial commit, and no changes are made to the commit
> message.
>
> Changing the behavior would break that perfectly reasonable,
> widely-practised workflow, and _that_ would be 100% not ok.

I'm not against that workflow - I'm not forcing anybody to change the commit message. The thing I'm talking about is roughly that it should NOT treat the result of command ":q" the same way as the command ":wq".

For example IF there is a variable somewhere where message is stored: For new commit message it is NULL and when commit is ready it is set up to "message". We check If it is not empty and not NULL (:wq executed), then we can commit. For "prepopulated" messages this variable can be is set to "message" from the beginning, so it doesn't matter if you're writing changes or not the check will always be true, so we can commit. If the implementation would be like this, then I would propose to clean up this variable after the prepopulated message is populated, so it won't use it blindly, but only after you're overwriting the message (even if the message is not changed).

Erik Faye-Lund· Feb 2, 2010, 22:03 UTC · re: Eugene Sajine · lore

Re: [BUG] - "git commit --amend" commits, when exiting the editor with no changes written

On Tue, Feb 2, 2010 at 10:56 PM, Eugene Sajine <euguess@gmail.com> wrote:
Show 14 quoted lines
>>
>> As has already been pointed out, an extremely common workflow for "git
>> commit --amend" is to include additional changes that were accidentally
>> overlooked  in the initial commit, and no changes are made to the commit
>> message.
>>
>> Changing the behavior would break that perfectly reasonable,
>> widely-practised workflow, and _that_ would be 100% not ok.
>
> I'm not against that workflow - I'm not forcing anybody to change the
> commit message. The thing I'm talking about is roughly that it should
> NOT treat the result of command ":q" the same way as the command
> ":wq".
>

Quite a lot of text-editors on prevents a user from saving when the buffer is unchanged, so I don't think this would actually work. Keep in mind that vi is not the only editor one can use to edit commit messages.

-- 
Erik "kusma" Faye-Lund
Wincent Colaiuta· Feb 2, 2010, 22:06 UTC · re: Eugene Sajine · lore

Re: [BUG] - "git commit --amend" commits, when exiting the editor with no changes written

El 02/02/2010, a las 22:56, Eugene Sajine escribió:
Show 16 quoted lines
>>
>> As has already been pointed out, an extremely common workflow for  
>> "git
>> commit --amend" is to include additional changes that were  
>> accidentally
>> overlooked  in the initial commit, and no changes are made to the  
>> commit
>> message.
>>
>> Changing the behavior would break that perfectly reasonable,
>> widely-practised workflow, and _that_ would be 100% not ok.
>
> I'm not against that workflow - I'm not forcing anybody to change the
> commit message. The thing I'm talking about is roughly that it should
> NOT treat the result of command ":q" the same way as the command
> ":wq".

Why should Git care about how you exited from your EDITOR? All it should care about is the contents of the commit message, and the exit code of the editor.

Show 7 quoted lines
> For example IF there is a variable somewhere where message is stored:
> For new commit message it is NULL and when commit is ready it is set
> up to "message". We check If it is not empty and not NULL (:wq
> executed),  then we can commit.
> For "prepopulated" messages this variable can be is set to "message"
> from the beginning, so it doesn't matter if you're writing changes or
> not the check will always be true, so we can commit.

I don't understand what change you're asking for, as things already work as you've just described (the "variable" is the file, .git/ COMMIT_EDITMSG).

> If the implementation would be like this, then I would propose to
> clean up this variable after the prepopulated message is populated, so
> it won't use it blindly, but only after you're overwriting the message
> (even if the message is not changed).

Here is where you've lost me. Not sure what you mean here, nor how you're going to maintain the existing behavior for all of the people who have grown used to it over a long, long period of time.

Wincent
Eugene Sajine· Feb 2, 2010, 22:31 UTC · re: Wincent Colaiuta · lore

Re: [BUG] - "git commit --amend" commits, when exiting the editor with no changes written

>
> Why should Git care about how you exited from your EDITOR? All it should
> care about is the contents of the commit message, and the exit code of the
> editor.

It doesn't matter which editor is used. The only thing that matters is that you saving or not saving on exit from editor. If not saving - abort the action.

Show 11 quoted lines
>
>> For example IF there is a variable somewhere where message is stored:
>> For new commit message it is NULL and when commit is ready it is set
>> up to "message". We check If it is not empty and not NULL (:wq
>> executed),  then we can commit.
>> For "prepopulated" messages this variable can be is set to "message"
>> from the beginning, so it doesn't matter if you're writing changes or
>> not the check will always be true, so we can commit.
>
> I don't understand what change you're asking for, as things already work as
> you've just described (the "variable" is the file, .git/COMMIT_EDITMSG).
So, i was right in my assumption about how it is implemented - good;)
Show 9 quoted lines
>
>> If the implementation would be like this, then I would propose to
>> clean up this variable after the prepopulated message is populated, so
>> it won't use it blindly, but only after you're overwriting the message
>> (even if the message is not changed).
>
> Here is where you've lost me. Not sure what you mean here, nor how you're
> going to maintain the existing behavior for all of the people who have grown
> used to it over a long, long period of time.
Let me explain it like this (sorry for syntax):
new commit:

COMMIT_EDITMSG = NULL; $ git commit

in background: $EDITOR .git/COMMIT_EDITMSG

If saved/written COMMIT_EDITMSG=message

if (COMMIT_EDITMSG != NULL && !COMMIT_EDITMSG.equalsIgnoreCase("") ) { commit(); } else abort();

==================== amend or rebase: COMMIT_EDITMSG = "message from commit or rebase sequence"; $ git commit --amend

in background: $EDITOR .git/COMMIT_EDITMSG

COMMIT_EDITMSG = NULL <======== this i think is missing. We have to clean it after it is populated in the editor.

If written back by save: COMMIT_EDITMSG="message from commit or rebase sequence" (it was not changed, but saved/written)

If not saved, the check below will abort the action

if (COMMIT_EDITMSG != NULL && !COMMIT_EDITMSG.equalsIgnoreCase("") ) { commit(); } else abort();

Is that a better explanation?

So, the only change from user perspective will be that you will have always save/write your changes/rebase sequences (even if you don't change the message) for the program to proceed with commit amend/rebase or whatever the same way as for the simple commit.

Thanks, Eugene

Avery Pennarun· Feb 2, 2010, 22:35 UTC · re: Eugene Sajine · lore

Re: [BUG] - "git commit --amend" commits, when exiting the editor with no changes written

Show 9 quoted lines
> amend or rebase:
> COMMIT_EDITMSG = "message from commit or rebase sequence";
> $ git commit --amend
>
> in background:
> $EDITOR .git/COMMIT_EDITMSG
>
> COMMIT_EDITMSG = NULL    <======== this i think is missing. We have to
> clean it after it is populated in the editor.

That's not going to work. Most editors will get quite angry at you if you modify the file while the editor has it open. Furthermore, how do you know how long to wait after launching the editor but before deleting the file?

Like I suggested earlier, monitoring the mtime of the file to see if it has changed after the editor exits, even if the file itself is identical to what it was, might work. This feature would have to be optional in order to not confuse existing users, and not annoy users of editors (like my favourite, joe) which don't save-on-exit if the file hasn't changed. But I think it might be valuable to some people nevertheless. And if it became popular, perhaps it could become the default in some future version of git (after giving people enough notice, etc, etc).

So you might want to submit a patch to implement an option like that.
Avery
Eugene Sajine· Feb 2, 2010, 23:02 UTC · re: Avery Pennarun · lore

Re: [BUG] - "git commit --amend" commits, when exiting the editor with no changes written

On Tue, Feb 2, 2010 at 5:35 PM, Avery Pennarun <apenwarr@gmail.com> wrote:
Show 14 quoted lines
>> amend or rebase:
>> COMMIT_EDITMSG = "message from commit or rebase sequence";
>> $ git commit --amend
>>
>> in background:
>> $EDITOR .git/COMMIT_EDITMSG
>>
>> COMMIT_EDITMSG = NULL    <======== this i think is missing. We have to
>> clean it after it is populated in the editor.
>
> That's not going to work.  Most editors will get quite angry at you if
> you modify the file while the editor has it open.  Furthermore, how do
> you know how long to wait after launching the editor but before
> deleting the file?

I'm not insisting on this approach by any mean - i was trying to explain what is the problem i consider to be a bug.

>
> Like I suggested earlier, monitoring the mtime of the file to see if
> it has changed after the editor exits, even if the file itself is
> identical to what it was, might work.
Anything that would work;)
Show 7 quoted lines
>This feature would have to be
> optional in order to not confuse existing users, and not annoy users
> of editors (like my favourite, joe) which don't save-on-exit if the
> file hasn't changed.  But I think it might be valuable to some people
> nevertheless.  And if it became popular, perhaps it could become the
> default in some future version of git (after giving people enough
> notice, etc, etc).

But here I totally disagree, because i don't understand what is so new and confusing in the workflow I'm talking about? Why you're not confused that you MUST save before exiting your EDITOR in order to be able to commit (else it will fail), but the same workflow suddenly becomes confusing when you doing "commit --amend" or especially "rebase -i" ???

Frankly, I think that "rebase -i" is the best example when exiting without save should abort, not proceed.

Show 5 quoted lines
>
> So you might want to submit a patch to implement an option like that.
>
> Avery
>

I wish i could - but, unfortunately, I'm as far from C as from the Sun (star;)). I'm developing a little bit in Java, but can't do C.

Thanks, Eugene

Johannes Schindelin· Feb 2, 2010, 23:15 UTC · re: Eugene Sajine · lore

Re: [BUG] - "git commit --amend" commits, when exiting the editor with no changes written

Hi,
On Tue, 2 Feb 2010, Eugene Sajine wrote:
> Frankly, I think that "rebase -i" is the best example when exiting 
> without save should abort, not proceed.

That would be horrible! If the list of commits I want to apply happens to be exactly what I want without needing any reordering/removing, then it would abort, according to you.

Ciao, Dscho

Eugene Sajine· Feb 2, 2010, 23:27 UTC · re: Johannes Schindelin · lore

Re: [BUG] - "git commit --amend" commits, when exiting the editor with no changes written

Show 7 quoted lines
>> Frankly, I think that "rebase -i" is the best example when exiting
>> without save should abort, not proceed.
>
> That would be horrible! If the list of commits I want to apply happens to
> be exactly what I want without needing any reordering/removing, then it
> would abort, according to you.
>

No! It wouldn't if you save instead of simply exiting, if you type :wq instead :q

I'm obviously failing to express my self ...;)

NOTHING is supposed to change for you except that you have to save on exit, write the file, confirm changes or absence of such, agree with generated content of rebase sequence. That is all i'm talking about.

You MUST save the file when you do commit - right? Because this file COMMIT_EDITMSG is empty. It will fail to commit otherwise - right? The problem is that this file is NOT empty when there is a prepopulated message like rebase -i sequence of commits or message from commit being amended. Therefore, you can exit without saving and the action will still happen - which is wrong and not consistent with "commit"!

Exit without saving for me means FORGET what i was doing here . So, it is obvious, that when I'm doing rebase -i and working with the list of commits exit without saving should mean - FORGET about rebase.

Thanks, Eugene

Avery Pennarun· Feb 2, 2010, 23:40 UTC · re: Eugene Sajine · lore

Re: [BUG] - "git commit --amend" commits, when exiting the editor with no changes written

On Tue, Feb 2, 2010 at 6:02 PM, Eugene Sajine <euguess@gmail.com> wrote:
Show 15 quoted lines
> On Tue, Feb 2, 2010 at 5:35 PM, Avery Pennarun <apenwarr@gmail.com> wrote:
>>This feature would have to be
>> optional in order to not confuse existing users, and not annoy users
>> of editors (like my favourite, joe) which don't save-on-exit if the
>> file hasn't changed.  But I think it might be valuable to some people
>> nevertheless.  And if it became popular, perhaps it could become the
>> default in some future version of git (after giving people enough
>> notice, etc, etc).
>
> But here I totally disagree, because i don't understand what is so new
> and confusing in the workflow I'm talking about?
> Why you're not confused that you MUST save before exiting your EDITOR
> in order to be able to commit (else it will fail), but the same
> workflow suddenly becomes confusing when you doing "commit --amend" or
> especially "rebase -i" ???
I think you're missing the point of people's objections.

It's pretty clear that the current behaviour is inconsistent and confusing to new users:

- If you make a new commit and change your mind, abort your editor:
commit is aborted.
- If you amend a commit and change your mind, abort your editor:
commit gets *replaced* and newbies don't know how to get it back.
Argh!

However, whether it's inconsistent and confusing is unfortunately not the question. The question is what can you do to possibly improve things while a) not making backwards-incompatible changes, b) not making things even more inconsistent, and c) not annoying existing long-term users who like it fine the way it is. (I use the current behaviour and I even kind of like it now that I understand it. But I still think it's confusing and admit there may be a better way.)

git development is kind of like working at Intel or Microsoft. If you want to achieve world domination, you don't make new versions that are incompatible with old ones, no matter how stupid you think the old one was.

You can however add *new* stuff. That's why I suggested adding an option. You could even make it a config option so you only have to set it once (just like setting your preferred editor).

> I wish i could - but, unfortunately, I'm as far from C as from the Sun
> (star;)). I'm developing a little bit in Java, but can't do C.
There's no better time to learn :)
Have fun,
Avery
Larry D'Anna· Feb 3, 2010, 06:15 UTC · re: Avery Pennarun · lore

Re: [BUG] - "git commit --amend" commits, when exiting the editor with no changes written

* Avery Pennarun (apenwarr@gmail.com) [100202 18:41]:
> You can however add *new* stuff.  That's why I suggested adding an
> option.  You could even make it a config option so you only have to
> set it once (just like setting your preferred editor).
Or, he can set his $EDITOR to a script that checks the mtime.
    --larry
Jeff King· Feb 3, 2010, 09:31 UTC · re: Larry D'Anna · lore

Re: [BUG] - "git commit --amend" commits, when exiting the editor with no changes written

On Wed, Feb 03, 2010 at 01:15:35AM -0500, Larry D'Anna wrote:
Show 6 quoted lines
> * Avery Pennarun (apenwarr@gmail.com) [100202 18:41]:
> > You can however add *new* stuff.  That's why I suggested adding an
> > option.  You could even make it a config option so you only have to
> > set it once (just like setting your preferred editor).
> 
> Or, he can set his $EDITOR to a script that checks the mtime.
Agreed. It could even look like this:
-- >8 --
$ cat >vi-must-edit <<'EOF'
#!/usr/bin/perl
sub mtime { (stat $_[0])[9] }
my $old = mtime($ARGV[-1]);
system('vi', @ARGV) and die "vi failed";
if (mtime($ARGV[-1]) == $old) {
  print STDERR "Aborting unmodified message\n";
  exit 1;
}
exit 0
EOF
$ chmod +x vi-must-edit
$ GIT_EDITOR=$PWD/vi-must-edit; export GIT_EDITOR
$ git commit --amend ;# and then quit the editor immediately
Aborting unmodified message
error: There was a problem with the editor 'vi-must-edit'.
Please supply the message using either -m or -F option.
-- 8< --

Do note, though, that there is a subtle race condition in using the mtime. If you make a change and save within a second of the editor starting, it will be ignored. To be thorough, you would probably want to check the size and inode, as well.

-Peff
Jeff King· Feb 3, 2010, 10:15 UTC · re: Jeff King · lore

Re: [BUG] - "git commit --amend" commits, when exiting the editor with no changes written

On Wed, Feb 03, 2010 at 04:31:50AM -0500, Jeff King wrote:
> > Or, he can set his $EDITOR to a script that checks the mtime.
> 
> Agreed. It could even look like this:

Also, while we are talking about editor-specific hacks, if you are using vim you can use ":cq" to exit with an error code.

-Peff
Avery Pennarun· Feb 3, 2010, 18:19 UTC · re: Jeff King · lore

Re: [BUG] - "git commit --amend" commits, when exiting the editor with no changes written

On Wed, Feb 3, 2010 at 4:31 AM, Jeff King <peff@peff.net> wrote:
Show 7 quoted lines
> On Wed, Feb 03, 2010 at 01:15:35AM -0500, Larry D'Anna wrote:
>> * Avery Pennarun (apenwarr@gmail.com) [100202 18:41]:
>> > You can however add *new* stuff.  That's why I suggested adding an
>> > option.  You could even make it a config option so you only have to
>> > set it once (just like setting your preferred editor).
>>
>> Or, he can set his $EDITOR to a script that checks the mtime.
Nice.
> Do note, though, that there is a subtle race condition in using the
> mtime. If you make a change and save within a second of the editor
> starting, it will be ignored. To be thorough, you would probably want to
> check the size and inode, as well.
Or just the file contents.
Avery
Johannes Schindelin· Feb 2, 2010, 23:00 UTC · re: Eugene Sajine · lore

Re: [BUG] - "git commit --amend" commits, when exiting the editor with no changes written

Hi,
On Tue, 2 Feb 2010, Eugene Sajine wrote:
[somebody wrote this, I suspect it was Wincent]
Show 8 quoted lines
>
> > Why should Git care about how you exited from your EDITOR? All it 
> > should care about is the contents of the commit message, and the exit 
> > code of the editor.
> 
> It doesn't matter which editor is used. The only thing that matters is 
> that you saving or not saving on exit from editor. If not saving - abort 
> the action.

No. If not saving, then it means that the user was fine with the message as-is.

Remember, --amend is usually for amending the _content_ of the patch, not so much the message.

Of course, you could come up with wonderfully complicated strategies such as "if nothing was added, then an unmodified commit message means 'abort', else it means 'go ahead'", but I hope that you agree that this would be very bad from the users' POV: it would be utterly confusing.

Ciao, Dscho

Eugene Sajine· Feb 2, 2010, 23:34 UTC · re: Johannes Schindelin · lore

Re: [BUG] - "git commit --amend" commits, when exiting the editor with no changes written

> Of course, you could come up with wonderfully complicated strategies such
> as "if nothing was added, then an unmodified commit message means 'abort',
> else it means 'go ahead'", but I hope that you agree that this would be
> very bad from the users' POV: it would be utterly confusing.

No complicated strategies necessary. I'm not talking about modified or not modified commit message. I'm not forcing anybody to modify commit message in order to proceed with commit --amend. I'm talking about consistent behaviour of main operations:

Commit - you must save on exit from your editor, simply exiting will abort.

Same should be applied to rebase -i and of course to commit --amend as it is still commit operation.

Even if the text is OK or you agree with generated sequence of rebase
- confirm that explicitly by saving on exit from the editor, else it
should abort. Pretty obvious to me...;)

Thanks, 'Eugene

Erik Faye-Lund· Feb 2, 2010, 23:40 UTC · re: Eugene Sajine · lore

Re: [BUG] - "git commit --amend" commits, when exiting the editor with no changes written

On Wed, Feb 3, 2010 at 12:34 AM, Eugene Sajine <euguess@gmail.com> wrote:
Show 13 quoted lines
>> Of course, you could come up with wonderfully complicated strategies such
>> as "if nothing was added, then an unmodified commit message means 'abort',
>> else it means 'go ahead'", but I hope that you agree that this would be
>> very bad from the users' POV: it would be utterly confusing.
>
> No complicated strategies necessary.
> I'm not talking about modified or not modified commit message. I'm not
> forcing anybody to modify commit message in order to proceed with
> commit --amend.
> I'm talking about consistent behaviour of main operations:
>
> Commit - you must save on exit from your editor, simply exiting will abort.
>

This is where you're wrong. The reason why commit (without --amend) is aborting when you don't save is that the default commit message is empty, not that it wasn't saved. In fact, just saving still makes in abort. You need to add something to the commit message and then save to get it to perform the commit.

> Same should be applied to rebase -i and of course to commit --amend as
> it is still commit operation.
>

"rebase -i" and "commit --amend" already has the exact same logic as commit without ammend in this regard - they abort if the buffer is empty.

-- 
Erik "kusma" Faye-Lund
Eugene Sajine· Feb 2, 2010, 23:48 UTC · re: Erik Faye-Lund · lore

Re: [BUG] - "git commit --amend" commits, when exiting the editor with no changes written

Show 18 quoted lines
>
> This is where you're wrong. The reason why commit (without --amend) is
> aborting when you don't save is that the default commit message is
> empty, not that it wasn't saved. In fact, just saving still makes in
> abort. You need to add something to the commit message and then save
> to get it to perform the commit.
>
>> Same should be applied to rebase -i and of course to commit --amend as
>> it is still commit operation.
>>
>
> "rebase -i" and "commit --amend" already has the exact same logic as
> commit without ammend in this regard - they abort if the buffer is
> empty.
>
> --
> Erik "kusma" Faye-Lund
>

Thank you, but I'm not wrong, as i described the exact same thing somewhere in my second or third message, without even knowing the implementation details. I understand the reason WHY it is like it is, i just don't like it as it is inconsistent and IMHO incorrect.

Thanks, Eugene

Erik Faye-Lund· Feb 3, 2010, 00:16 UTC · re: Eugene Sajine · lore

Re: [BUG] - "git commit --amend" commits, when exiting the editor with no changes written

On Wed, Feb 3, 2010 at 12:48 AM, Eugene Sajine <euguess@gmail.com> wrote:
Show 24 quoted lines
>>
>> This is where you're wrong. The reason why commit (without --amend) is
>> aborting when you don't save is that the default commit message is
>> empty, not that it wasn't saved. In fact, just saving still makes in
>> abort. You need to add something to the commit message and then save
>> to get it to perform the commit.
>>
>>> Same should be applied to rebase -i and of course to commit --amend as
>>> it is still commit operation.
>>>
>>
>> "rebase -i" and "commit --amend" already has the exact same logic as
>> commit without ammend in this regard - they abort if the buffer is
>> empty.
>>
>> --
>> Erik "kusma" Faye-Lund
>>
>
> Thank you, but I'm not wrong, as i described the exact same thing
> somewhere in my second or third message, without even knowing the
> implementation details. I understand the reason WHY it is like it is,
> i just don't like it as it is inconsistent and IMHO incorrect.
>

But you are. The interface isn't inconsistent. Your model of how it works however, is wrong.

-- 
Erik "kusma" Faye-Lund
Eugene Sajine· Feb 3, 2010, 00:55 UTC · re: Erik Faye-Lund · lore

Re: [BUG] - "git commit --amend" commits, when exiting the editor with no changes written

On Tue, Feb 2, 2010 at 7:16 PM, Erik Faye-Lund <kusmabite@googlemail.com> wrote:
Show 32 quoted lines
> On Wed, Feb 3, 2010 at 12:48 AM, Eugene Sajine <euguess@gmail.com> wrote:
>>>
>>> This is where you're wrong. The reason why commit (without --amend) is
>>> aborting when you don't save is that the default commit message is
>>> empty, not that it wasn't saved. In fact, just saving still makes in
>>> abort. You need to add something to the commit message and then save
>>> to get it to perform the commit.
>>>
>>>> Same should be applied to rebase -i and of course to commit --amend as
>>>> it is still commit operation.
>>>>
>>>
>>> "rebase -i" and "commit --amend" already has the exact same logic as
>>> commit without ammend in this regard - they abort if the buffer is
>>> empty.
>>>
>>> --
>>> Erik "kusma" Faye-Lund
>>>
>>
>> Thank you, but I'm not wrong, as i described the exact same thing
>> somewhere in my second or third message, without even knowing the
>> implementation details. I understand the reason WHY it is like it is,
>> i just don't like it as it is inconsistent and IMHO incorrect.
>>
>
> But you are. The interface isn't inconsistent. Your model of how it
> works however, is wrong.
>
> --
> Erik "kusma" Faye-Lund
>

I'm sorry, but I don't get in which part my model is so wrong? The only difference i see between your and my explanation is that you mentioned the fact that simply saving the pregenerated new commit message is not working because there is no meaningful (uncommented) string inserted into the template. But this doesn't change the problem it self, so I don't think it is relevant.

What is relevant though is that in current workflow with "rebase -i" you have to remove! all commits listed for rebase and save! the file in order to cancel! rebase operation. This is very strange user interaction model IMHO

Instead of being explicit in applying the changes, we are explicit in cancelling the operation. Very strange...

Thanks, Eugene

SZEDER Gábor· Feb 3, 2010, 01:59 UTC · re: Eugene Sajine · lore

Re: [BUG] - "git commit --amend" commits, when exiting the editor with no changes written

On Tue, Feb 02, 2010 at 07:55:31PM -0500, Eugene Sajine wrote:
Show 48 quoted lines
> On Tue, Feb 2, 2010 at 7:16 PM, Erik Faye-Lund <kusmabite@googlemail.com> wrote:
> > On Wed, Feb 3, 2010 at 12:48 AM, Eugene Sajine <euguess@gmail.com> wrote:
> >>>
> >>> This is where you're wrong. The reason why commit (without --amend) is
> >>> aborting when you don't save is that the default commit message is
> >>> empty, not that it wasn't saved. In fact, just saving still makes in
> >>> abort. You need to add something to the commit message and then save
> >>> to get it to perform the commit.
> >>>
> >>>> Same should be applied to rebase -i and of course to commit --amend as
> >>>> it is still commit operation.
> >>>>
> >>>
> >>> "rebase -i" and "commit --amend" already has the exact same logic as
> >>> commit without ammend in this regard - they abort if the buffer is
> >>> empty.
> >>>
> >>> --
> >>> Erik "kusma" Faye-Lund
> >>>
> >>
> >> Thank you, but I'm not wrong, as i described the exact same thing
> >> somewhere in my second or third message, without even knowing the
> >> implementation details. I understand the reason WHY it is like it is,
> >> i just don't like it as it is inconsistent and IMHO incorrect.
> >>
> >
> > But you are. The interface isn't inconsistent. Your model of how it
> > works however, is wrong.
> >
> > --
> > Erik "kusma" Faye-Lund
> >
> 
> I'm sorry, but I don't get in which part my model is so wrong? The only
> difference i see between your and my explanation is that you mentioned the
> fact that simply saving the pregenerated new commit message is not working
> because there is no meaningful (uncommented) string inserted into the
> template. But this
> doesn't change the problem it self, so I don't think it is relevant.
> 
> What is relevant though is that in current workflow with "rebase -i"
> you have to remove! all
> commits listed for rebase and save! the file in order to cancel! rebase
> operation. This is very strange user interaction model IMHO
> 
> Instead of being explicit in applying the changes, we are explicit in
> cancelling the operation. Very strange...

No. In case of rebase -i, exiting the editor is being explicit in "the todo file now contains what you should do, so do it". And if the todo file happens to be empty, then there is nothing to do, so do nothing. The same applies to commit and commit --amend: exiting the editor is being explicit in "the COMMIT_MSG file now contains what you should put in the commit message". But not having a commit message is a really bad idea, therefore git complains and doesn't do that (unless overridden by some command line option).

Best, Gábor

Matthieu Moy· Feb 3, 2010, 07:34 UTC · re: SZEDER Gábor · lore

Re: [BUG] - "git commit --amend" commits, when exiting the editor with no changes written

SZEDER Gábor <szeder@ira.uka.de> writes:
> No.  In case of rebase -i, exiting the editor is being explicit in
> "the todo file now contains what you should do, so do it".  And if the
> todo file happens to be empty, then there is nothing to do, so do
> nothing.

It's not true. The todolist is the list of things to do after you rewind your branch to the destination branch. Strictly speaking, an empty todolist should mean to drop all the patches (like a todolist with just one line would mean to drop all the others). But a user never wants to do that (otherwise, "git reset" would be the right command), so "git rebase -i" considers it as a special case.

My 2 cents,
-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
Johannes Schindelin· Feb 3, 2010, 09:08 UTC · re: Matthieu Moy · lore

aborting rebase -i right at the start, was Re: [BUG] - "git commit --amend" commits, when exiting the editor with no changes written

Hi,
On Wed, 3 Feb 2010, Matthieu Moy wrote:
Show 5 quoted lines
> Strictly speaking, an empty [rebase -i] todolist should mean to drop all 
> the patches (like a todolist with just one line would mean to drop all 
> the others). But a user never wants to do that (otherwise, "git reset" 
> would be the right command), so "git rebase -i" considers it as a 
> special case.

Actually, it is a design bug, but it was the only sane way I could think of aborting the rebase.

Note that there _are_ users who want to do that ("let me see what commits I have, ah, oh, okay, I want none of them"). I am one of those. That's where the underadvertised "noop" command comes in.

Ciao, Dscho

SZEDER Gábor· Feb 3, 2010, 09:41 UTC · re: Johannes Schindelin · lore

Re: aborting rebase -i right at the start, was Re: [BUG] - "git commit --amend" commits, when exiting the editor with no changes written

On Wed, Feb 03, 2010 at 10:08:50AM +0100, Johannes Schindelin wrote:
Show 15 quoted lines
> Hi,
> 
> On Wed, 3 Feb 2010, Matthieu Moy wrote:
> 
> > Strictly speaking, an empty [rebase -i] todolist should mean to drop all 
> > the patches (like a todolist with just one line would mean to drop all 
> > the others). But a user never wants to do that (otherwise, "git reset" 
> > would be the right command), so "git rebase -i" considers it as a 
> > special case.
> 
> Actually, it is a design bug, but it was the only sane way I could think 
> of aborting the rebase.
> 
> Note that there _are_ users who want to do that ("let me see what commits 
> I have, ah, oh, okay, I want none of them"). I am one of those.

I regularly mess up the todo file so badly that I don't want to bother with undo but rather start over from the beginning.

Best, Gábor

Eugene Sajine· Feb 3, 2010, 16:02 UTC · re: SZEDER Gábor · lore

Re: aborting rebase -i right at the start, was Re: [BUG] - "git commit --amend" commits, when exiting the editor with no changes written

2010/2/3 SZEDER Gábor <szeder@ira.uka.de>:
Show 25 quoted lines
> On Wed, Feb 03, 2010 at 10:08:50AM +0100, Johannes Schindelin wrote:
>> Hi,
>>
>> On Wed, 3 Feb 2010, Matthieu Moy wrote:
>>
>> > Strictly speaking, an empty [rebase -i] todolist should mean to drop all
>> > the patches (like a todolist with just one line would mean to drop all
>> > the others). But a user never wants to do that (otherwise, "git reset"
>> > would be the right command), so "git rebase -i" considers it as a
>> > special case.
>>
>> Actually, it is a design bug, but it was the only sane way I could think
>> of aborting the rebase.
>>
>> Note that there _are_ users who want to do that ("let me see what commits
>> I have, ah, oh, okay, I want none of them"). I am one of those.
>
> I regularly mess up the todo file so badly that I don't want to bother
> with undo but rather start over from the beginning.
>
>
> Best,
> Gábor
>
>

I already described the solution which i think should be implemented in old thread I will just repeat it here for easier tracking:

The solution should be as Avery proposed - to monitor file modification timestamp as well as the content. Timestamp should be remembered by git when editor is fired up and then checked when you're exiting the editor. If it is the same - it means that user canceled the whole operation by *not* confirming the file content by save. If the timestamp is bigger than the saved one and the message is not empty - we can proceed.

I.e. in case of rebase -i you will be able to abort the operation by simply exiting the editor without save in any state at any moment and start from the beginning.

Thanks, Eugene

Matthieu Moy· Feb 3, 2010, 07:31 UTC · re: Eugene Sajine · lore

Re: [BUG] - "git commit --amend" commits, when exiting the editor with no changes written

Eugene Sajine <euguess@gmail.com> writes:
> Thank you, but I'm not wrong, as i described the exact same thing
> somewhere in my second or third message, without even knowing the
> implementation details. I understand the reason WHY it is like it is,
> i just don't like it as it is inconsistent and IMHO incorrect.
Just try:
Create a file.
1) Launch emacs, save and quit.
2) Launch emacs, don't save, and quit.
>From outside, it's EXACTLY the same thing. In the first case, emacs

will just tell you "no change need to be saved" and quit, in the second, it'll quit. Try deleting the file in the meantime, it won't change the behavior.

Now, what would you do about this? Ignore Emacs and force people to use vi?

People have been spending a whole thread to explain you that it's not going to work. I think it'll either be time to acknowledge that, or to learn C and write a patch. Or perhaps try to write it in Java to understand why it doesn't work.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
Eugene Sajine· Feb 3, 2010, 15:45 UTC · re: Matthieu Moy · lore

Re: [BUG] - "git commit --amend" commits, when exiting the editor with no changes written

On Wed, Feb 3, 2010 at 2:31 AM, Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> wrote:

Show 30 quoted lines
> Eugene Sajine <euguess@gmail.com> writes:
>
>> Thank you, but I'm not wrong, as i described the exact same thing
>> somewhere in my second or third message, without even knowing the
>> implementation details. I understand the reason WHY it is like it is,
>> i just don't like it as it is inconsistent and IMHO incorrect.
>
> Just try:
>
> Create a file.
> 1) Launch emacs, save and quit.
> 2) Launch emacs, don't save, and quit.
>
> From outside, it's EXACTLY the same thing. In the first case, emacs
> will just tell you "no change need to be saved" and quit, in the
> second, it'll quit. Try deleting the file in the meantime, it won't
> change the behavior.
>
> Now, what would you do about this? Ignore Emacs and force people to
> use vi?
>
> People have been spending a whole thread to explain you that it's not
> going to work. I think it'll either be time to acknowledge that, or to
> learn C and write a patch. Or perhaps try to write it in Java to
> understand why it doesn't work.
>
> --
> Matthieu Moy
> http://www-verimag.imag.fr/~moy/
>

Tell me you're not serious, please;) I'm working with computers for more than 20 years and I do understand how editors are opening files. If you are serious though...:

Please, do not assume i don't understand that file opened in editor and saved with no changes will be the same as not saved.

Please, do not assume i don't understand that current implementation does not allow to correctly abort by simply quiting the editor, because it uses the file content only to verify if it can proceed.

I will stop trying to explain the problem here, but the solution, that i think should work:

The solution should be as Avery proposed - to monitor file modification timestamp as well as the content. Timestamp should be remembered by git when editor is fired up and then checked when you're exiting the editor. If it is the same - it means that user canceled the whole operation by not confirming the file content by save. If the timestamp is bigger than the saved one and the message is not empty - we can proceed. Something like that.

And finally:

I DO highly appreciate everybody's time spent on every discussion and particularly this one. I ask you to excuse me if i was failing to properly express my thoughts or if i was too emotional at times.

Thanks a lot, Eugene

Jonathan Nieder· Feb 3, 2010, 17:51 UTC · re: Eugene Sajine · lore

Re: [BUG] - "git commit --amend" commits, when exiting the editor with no changes written

Eugene Sajine wrote:
> On Wed, Feb 3, 2010 at 2:31 AM, Matthieu Moy
Show 10 quoted lines
>> Just try:
>>
>> Create a file.
>> 1) Launch emacs, save and quit.
>> 2) Launch emacs, don't save, and quit.
>>
>> From outside, it's EXACTLY the same thing. In the first case, emacs
>> will just tell you "no change need to be saved" and quit, in the
>> second, it'll quit. Try deleting the file in the meantime, it won't
>> change the behavior.
Show 6 quoted lines
> Please, do not assume i don't understand that file opened in editor
> and saved with no changes will be the same as not saved.
> 
> Please, do not assume i don't understand that current implementation
> does not allow to correctly abort by simply quiting the editor,
> because it uses the file content only to verify if it can proceed.

I’ll bite. Please reread Mattheiu’s message. Pay particular attention to the lines

	In the first case, emacs
	will just tell you "no change need to be saved" and quit, in the
	second, it'll quit.
and
	Try deleting the file in the meantime, it won't
	change the behavior.

and consider what this means about what is happening and the resulting stat() information.

It is a bit frustrating to read this thread because Mattheiu was not the first thing to say this (and many have mentioned other problems, too, such as people’s muscle memory), but you do not seem to respond to them. Instead, you keep repeating the same thing...

Hope that helps, Jonathan

Jonathan Nieder· Feb 3, 2010, 17:53 UTC · re: Jonathan Nieder · lore

Re: [BUG] - "git commit --amend" commits, when exiting the editor with no changes written

Jonathan Nieder wrote:
> It is a bit frustrating to read this thread because Mattheiu was not the
> first thing to say this
s/thing/person/
Sorry, Mattheiu. ;-)

Kind regards, Jonathan

Wincent Colaiuta· Feb 3, 2010, 18:21 UTC · re: Eugene Sajine · lore

Re: [BUG] - "git commit --amend" commits, when exiting the editor with no changes written

El 03/02/2010, a las 16:45, Eugene Sajine escribió:
Show 63 quoted lines
> On Wed, Feb 3, 2010 at 2:31 AM, Matthieu Moy
> <Matthieu.Moy@grenoble-inp.fr> wrote:
>> Eugene Sajine <euguess@gmail.com> writes:
>>
>>> Thank you, but I'm not wrong, as i described the exact same thing
>>> somewhere in my second or third message, without even knowing the
>>> implementation details. I understand the reason WHY it is like it  
>>> is,
>>> i just don't like it as it is inconsistent and IMHO incorrect.
>>
>> Just try:
>>
>> Create a file.
>> 1) Launch emacs, save and quit.
>> 2) Launch emacs, don't save, and quit.
>>
>> From outside, it's EXACTLY the same thing. In the first case, emacs
>> will just tell you "no change need to be saved" and quit, in the
>> second, it'll quit. Try deleting the file in the meantime, it won't
>> change the behavior.
>>
>> Now, what would you do about this? Ignore Emacs and force people to
>> use vi?
>>
>> People have been spending a whole thread to explain you that it's not
>> going to work. I think it'll either be time to acknowledge that, or  
>> to
>> learn C and write a patch. Or perhaps try to write it in Java to
>> understand why it doesn't work.
>>
>> --
>> Matthieu Moy
>> http://www-verimag.imag.fr/~moy/
>>
>
> Tell me you're not serious, please;) I'm working with computers for
> more than 20 years and I do understand how editors are opening files.
> If you are serious though...:
>
> Please, do not assume i don't understand that file opened in editor
> and saved with no changes will be the same as not saved.
>
> Please, do not assume i don't understand that current implementation
> does not allow to correctly abort by simply quiting the editor,
> because it uses the file content only to verify if it can proceed.
>
>
> I will stop trying to explain the problem here, but the solution, that
> i think should work:
>
> The solution should be as Avery proposed - to monitor file
> modification timestamp as well as the content.
> Timestamp should be remembered by git when editor is fired up and then
> checked when you're exiting the editor. If it is the same - it means
> that user canceled the whole operation by not confirming the file
> content by save. If the timestamp is bigger than the saved one and the
> message is not empty - we can proceed. Something like that.
>
> And finally:
>
> I DO highly appreciate everybody's time spent on every discussion and
> particularly this one. I ask you to excuse me if i was failing to
> properly express my thoughts or if i was too emotional at times.

Perhaps you could invest a little time actually reading what people have written. Your proposal won't work because in the case where the user wants to keep the commit message without any modifications many editors won't touch the file even if people explicitly save it.

Wincent
Eugene Sajine· Feb 3, 2010, 18:49 UTC · re: Wincent Colaiuta · lore

Re: [BUG] - "git commit --amend" commits, when exiting the editor with no changes written

On Wed, Feb 3, 2010 at 1:21 PM, Wincent Colaiuta <win@wincent.com> wrote:
Show 73 quoted lines
> El 03/02/2010, a las 16:45, Eugene Sajine escribió:
>
>> On Wed, Feb 3, 2010 at 2:31 AM, Matthieu Moy
>> <Matthieu.Moy@grenoble-inp.fr> wrote:
>>>
>>> Eugene Sajine <euguess@gmail.com> writes:
>>>
>>>> Thank you, but I'm not wrong, as i described the exact same thing
>>>> somewhere in my second or third message, without even knowing the
>>>> implementation details. I understand the reason WHY it is like it is,
>>>> i just don't like it as it is inconsistent and IMHO incorrect.
>>>
>>> Just try:
>>>
>>> Create a file.
>>> 1) Launch emacs, save and quit.
>>> 2) Launch emacs, don't save, and quit.
>>>
>>> From outside, it's EXACTLY the same thing. In the first case, emacs
>>> will just tell you "no change need to be saved" and quit, in the
>>> second, it'll quit. Try deleting the file in the meantime, it won't
>>> change the behavior.
>>>
>>> Now, what would you do about this? Ignore Emacs and force people to
>>> use vi?
>>>
>>> People have been spending a whole thread to explain you that it's not
>>> going to work. I think it'll either be time to acknowledge that, or to
>>> learn C and write a patch. Or perhaps try to write it in Java to
>>> understand why it doesn't work.
>>>
>>> --
>>> Matthieu Moy
>>> http://www-verimag.imag.fr/~moy/
>>>
>>
>> Tell me you're not serious, please;) I'm working with computers for
>> more than 20 years and I do understand how editors are opening files.
>> If you are serious though...:
>>
>> Please, do not assume i don't understand that file opened in editor
>> and saved with no changes will be the same as not saved.
>>
>> Please, do not assume i don't understand that current implementation
>> does not allow to correctly abort by simply quiting the editor,
>> because it uses the file content only to verify if it can proceed.
>>
>>
>> I will stop trying to explain the problem here, but the solution, that
>> i think should work:
>>
>> The solution should be as Avery proposed - to monitor file
>> modification timestamp as well as the content.
>> Timestamp should be remembered by git when editor is fired up and then
>> checked when you're exiting the editor. If it is the same - it means
>> that user canceled the whole operation by not confirming the file
>> content by save. If the timestamp is bigger than the saved one and the
>> message is not empty - we can proceed. Something like that.
>>
>> And finally:
>>
>> I DO highly appreciate everybody's time spent on every discussion and
>> particularly this one. I ask you to excuse me if i was failing to
>> properly express my thoughts or if i was too emotional at times.
>
> Perhaps you could invest a little time actually reading what people have
> written. Your proposal won't work because in the case where the user wants
> to keep the commit message without any modifications many editors won't
> touch the file even if people explicitly save it.
>
> Wincent
>
>
Thank you for clarifying that.

OK. I admit I got confused with the fact that some editors (vi, kwrite, gedit - i just tested them to be 100% sure) do allow you to hit save right after the file was opened and the timestamp will be changed, while emacs or open office writer or some others do not, if no changes has been made. Therefore there is no way to actually change the timestamp on the file from editors like emacs without actually changing something.

I'm very sorry for confusion, so, please, accept my apologies for being stubborn.

I hope it is clear that my only intention is to try to make git better and provide useful feedback and I'm sorry i didn't succeed this time.

Thanks, Eugene

Avery Pennarun· Feb 3, 2010, 18:57 UTC · re: Eugene Sajine · lore

Re: [BUG] - "git commit --amend" commits, when exiting the editor with no changes written

On Wed, Feb 3, 2010 at 1:49 PM, Eugene Sajine <euguess@gmail.com> wrote:
Show 7 quoted lines
> OK. I admit I got confused with the fact that some editors (vi,
> kwrite, gedit - i just tested them to be 100% sure) do allow you to
> hit save right after the file was opened and the timestamp will be
> changed, while emacs or open office writer or some others do not, if
> no changes has been made. Therefore there is no way to actually change
> the timestamp on the file from editors like emacs without actually
> changing something.

Of course, in such editors you could just hit "space; backspace" and then save. Sounds annoying? Well, so does deleting all the lines in the commit message just to make it *not* amend.

To reiterate what I said earlier: the mtime idea isn't even automatically a bad one. It's about as good as what currently exists, and the resulting rule (file content or mtime must be modified) is just as consistent as the current rule (file must be nonempty). It's also arguably easier for new users to understand.

But you can't just blindly change the system to always work in a different way. People depend on the current behaviour. Jeff King's script is a pretty cute solution that lets you have it your way with no changes to git, though.

Of course, this does open up the question of how to do any global UI design at all if a decision made once gets locked in forever. The reason git is hard for new users is that its UI is crazy and confusing, but a major reason it keeps gaining in popularity is that once you learn git, you stick with it, because you don't have to relearn it with every new version.

Have fun,
Avery
Eugene Sajine· Feb 3, 2010, 19:27 UTC · re: Avery Pennarun · lore

Re: [BUG] - "git commit --amend" commits, when exiting the editor with no changes written

> Of course, in such editors you could just hit "space; backspace" and
> then save.  Sounds annoying?  Well, so does deleting all the lines in
> the commit message just to make it *not* amend.

That was exactly the starting point of my thoughts.Currently it forces user to be explicit if he wants to cancel the operation (rebase -i, or amend), instead of forcing user to be explicit to proceed.

Show 6 quoted lines
>
> To reiterate what I said earlier: the mtime idea isn't even
> automatically a bad one.  It's about as good as what currently exists,
> and the resulting rule (file content or mtime must be modified) is
> just as consistent as the current rule (file must be nonempty).  It's
> also arguably easier for new users to understand.
OK. Agree on that.
>
> But you can't just blindly change the system to always work in a
> different way.  People depend on the current behaviour.

I understand that pretty well. My logic is that the most used command which is affecting the three is "git commit", therefore this command workflow is pretty natural for all users, but the workflow of commit --amend and rebase -i which are also affecting the three is inconsistent with commit. The user interaction is not the same.

> Jeff King's script is a pretty cute solution that lets you have it your way with
> no changes to git, though.
Thanks to Jeff, I'll try it.
>
> Of course, this does open up the question of how to do any global UI
> design at all if a decision made once gets locked in forever.
100% agree, there is such question.
> The reason git is hard for new users is that its UI is crazy and
> confusing, but a major reason it keeps gaining in popularity is that
> once you learn git, you stick with it, because you don't have to
> relearn it with every new version.

I just didn't realize that this can be such a drastic change for users as well as technical difficulties related to the emacs and similar editors.

Thanks, Eugene

Avery Pennarun· Feb 3, 2010, 19:54 UTC · re: Eugene Sajine · lore

Re: [BUG] - "git commit --amend" commits, when exiting the editor with no changes written

On Wed, Feb 3, 2010 at 2:27 PM, Eugene Sajine <euguess@gmail.com> wrote:
Show 16 quoted lines
>> To reiterate what I said earlier: the mtime idea isn't even
>> automatically a bad one.  It's about as good as what currently exists,
>> and the resulting rule (file content or mtime must be modified) is
>> just as consistent as the current rule (file must be nonempty).  It's
>> also arguably easier for new users to understand.
>
> OK. Agree on that.
>
>> But you can't just blindly change the system to always work in a
>> different way.  People depend on the current behaviour.
>
> I understand that pretty well. My logic is that the most used command
> which is affecting the three is "git commit", therefore this command
> workflow is pretty natural for all users, but the workflow of commit
> --amend and rebase -i which are also affecting the three is
> inconsistent with commit. The user interaction is not the same.

You can't agree with my first statement above and yet still call the commit --amend behaviour inconsistent with normal commit behaviour. It is 100% consistent, as others have already pointed out. Whether it is consistent is not the problem. Your insistence on that point is what's making people argue with you and is undermining your real point.

Have fun,
Avery
Matthieu Moy· Feb 3, 2010, 18:47 UTC · re: Eugene Sajine · lore

Re: [BUG] - "git commit --amend" commits, when exiting the editor with no changes written

Eugene Sajine <euguess@gmail.com> writes:
> Tell me you're not serious, please;) I'm working with computers for
> more than 20 years and I do understand how editors are opening files.
> If you are serious though...:

I'm damned serious. If you've been working for 20 years with computers, you probably understand what "no change need to be saved" mean.

> The solution should be as Avery proposed - to monitor file
> modification timestamp as well as the content.

Try it. I do mean "try it". I don't think anyone will be able to explain you better than it has already been done, but ... it doesn't work.

So, again, stop arguing, try it, and see by yourself. I promise public apologies if you prove me wrong with an actually working piece of code.

> Timestamp should be remembered by git when editor is fired up and then
> checked when you're exiting the editor.

Which doesn't work reliable, as you would have noticed by reading my message.

(and yes, I'm tired and angry after a long day of work ;-) ).
-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
Johannes Schindelin· Feb 2, 2010, 23:58 UTC · re: Eugene Sajine · lore

Re: [BUG] - "git commit --amend" commits, when exiting the editor with no changes written

Hi,
On Tue, 2 Feb 2010, Eugene Sajine wrote:
Show 7 quoted lines
> > Of course, you could come up with wonderfully complicated strategies 
> > such as "if nothing was added, then an unmodified commit message means 
> > 'abort', else it means 'go ahead'", but I hope that you agree that 
> > this would be very bad from the users' POV: it would be utterly 
> > confusing.
> 
> No complicated strategies necessary.

Sorry, your "if message was not saved, abort" is so out of line with any program I use that this would be highly confusing (maybe "complicated" was the wrong choice of words).

IMO it would not buy us anything over the current solution, which also requires the user to read the manual.

Ciao, Dscho

Eugene Sajine· Feb 3, 2010, 00:09 UTC · re: Johannes Schindelin · lore

Re: [BUG] - "git commit --amend" commits, when exiting the editor with no changes written

> Sorry, your "if message was not saved, abort" is so out of line with any
> program I use that this would be highly confusing (maybe "complicated" was
> the wrong choice of words).

Frankly, i think it is in line with "git commit" and that's the only thing that matters here as I have no idea what programs you're talking about. If your programs are working without making sure that user confirmed whatever he actually supposed to confirm, then you probably should revisit them;)

Thanks, Eugene

Johannes Schindelin· Feb 3, 2010, 09:04 UTC · re: Eugene Sajine · lore

Re: [BUG] - "git commit --amend" commits, when exiting the editor with no changes written

Hi,
On Tue, 2 Feb 2010, Eugene Sajine wrote:
Show 7 quoted lines
> > Sorry, your "if message was not saved, abort" is so out of line with 
> > any program I use that this would be highly confusing (maybe 
> > "complicated" was the wrong choice of words).
> 
> Frankly, i think it is in line with "git commit" and that's the only 
> thing that matters here as I have no idea what programs you're talking 
> about.

It's not. 'git commit' will abort if you delete all _and save_. So it is not the saving part, it is the deleting all part. And that's exactly how rebase -i works, or add -e: if you delete all and save, it aborts.

Ciao, Dscho

Paolo Bonzini· Feb 3, 2010, 09:46 UTC · re: Eugene Sajine · lore

Re: [BUG] - "git commit --amend" commits, when exiting the editor with no changes written

On 02/03/2010 01:09 AM, Eugene Sajine wrote:
Show 5 quoted lines
>> Sorry, your "if message was not saved, abort" is so out of line with any
>> program I use that this would be highly confusing (maybe "complicated" was
>> the wrong choice of words).
>
> Frankly, i think it is in line with "git commit"

The fundamental problem is this. The behavior of "git commit" can be explained in two ways: a) abort if not saved; b) abort if empty. (a) may be an easier guess from experience (you type ":q!" at the vi prompt), but (b) is clear from the error message ("commit has empty message--aborting"). All this is irrefutable.

Now, the scientific method to see which one is correct, would be to try other commands including "git commit --amend" and see which of the two hypotheses was right. It turns out to be that (b) is correct and consistent across various commands; instead you insist that everything else should be modified to conform to (a). Since git is already consistent, and since nobody can think of a way to implement your proposal, there is obvious resistence to any change in this area.

Paolo
Junio C Hamano· Feb 2, 2010, 23:44 UTC · re: Johannes Schindelin · lore

Re: [BUG] - "git commit --amend" commits, when exiting the editor with no changes written

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
Show 5 quoted lines
> No. If not saving, then it means that the user was fine with the message 
> as-is.
>
> Remember, --amend is usually for amending the _content_ of the patch, not 
> so much the message.

I very often use --amend to update only the message, so I would strongly disagree with the "usually".

But that doesn't change my conclusion ;-) It is immaterial if you amended the tree or the message. If either or both changed, we do need to create a new commit---there is no question about it, so which one is more often done does not matter. Eugene does not gain anything by "Remember"ing it.

What is more important is what to do when neither changed.

If you run "rebase -i" and leave the earlier "pick"s intact, it seems to try keeping the commits intact, instead of rewriting.

It is understandable if somebody wanted to keep using the old commit object when the committer timestamp is the _only_ thing that is different between the original and the amended commit, after "git commit --amend" is run. Essentially, the behaviour of "rebase -i" for earlier "pick"s comes from the same motivation.

Right now, if you run this in a loop:
    while hell has not frozen over
    do
	git reset && EDITOR=: git commit --amend
    done

it will keep generating almost identical commits, except for their committer timestamps. The very first one may have different committer identity from the original if you are amending somebody else's commit, but after that it will be all you over and over again, only at different times.

I am inclined to say that it is a feature that it updates the commit in such a case to record the _last time_ you touched the commit. You ran "commit --amend" and did not use the any published method to _abort_ the amend.

As mentioned already, @{1} is your friend. Regardless of the difference between @{0} and @{1}, it is a way to go back to the previous commit if you do not like the latest commit for whatever reason; the latest commit may have a "wrong tree", a "wrong message", or a "wrong committer timestamp".

Avery Pennarun· Feb 2, 2010, 21:18 UTC · re: Eugene Sajine · lore

Re: [BUG] - "git commit --amend" commits, when exiting the editor with no changes written

On Tue, Feb 2, 2010 at 3:47 PM, Eugene Sajine <euguess@gmail.com> wrote:
Show 10 quoted lines
> If you're just exiting the vi using :q without writing changes it will
> perform rebase! I'm 100% sure it is not OK.
>
> So, it seems that it will do that every time it has some
> *prepopulated* message in the editor, so it doesn't understand when
> changes are written and when they are not.
>
> I believe all those actions should be treated exactly the same way as
> the commit itself. If I'm writing/saving changes - it is good to go.
> If not - abort.

It might be sort of neat to check the mtime of the commit message file before and after launching the editor; then if you want, you could force-save the file even without any changes, and git would know that you want it changed.

Not sure I would like that behaviour better than the current way, actually, but it's an idea.

Avery

← back to recent threads