threads / patch / 21128

patchTeach 'rebase -i' the command "amend"

Subject: [PATCH] Teach 'rebase -i' the command "amend"

## tl;dr

13 messages between Oct 4, 2009 and Oct 6, 2009. Diffs are folded; open one to read it.

replies: 12people: 6as markdown or json

Björn Gustavsson· Oct 4, 2009, 19:06 UTC · lore

Make it easier to edit just the commit message for a commit using 'git rebase -i' by introducing the "amend" command.

Signed-off-by: Björn Gustavsson <bgustavsson@gmail.com>
---
Here is my first ever patch of git.

I find that I often use 'git rebase -i' just to correct typos in my commit messages or to fill in more details, so I thought it would be a good idea to add to add an "amend" command to 'git rebase -i'.

/Björn
 Documentation/git-rebase.txt  |    3 +++
 git-rebase--interactive.sh    |    9 +++++++++
 t/lib-rebase.sh               |    6 +++---
 t/t3404-rebase-interactive.sh |   14 ++++++++++++++
 4 files changed, 29 insertions(+), 3 deletions(-)
Show changes to 4 files +29 −3

Documentation/git-rebase.txt, git-rebase--interactive.sh, t/lib-rebase.sh, t/t3404-rebase-interactive.sh

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 0aefc34..28b90ec 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -368,6 +368,9 @@ By replacing the command "pick" with the command "edit", you can tell
 the files and/or the commit message, amend the commit, and continue
 rebasing.
 
+If you just want to edit the commit message for a commit, you can replace
+the command "pick" with the command "amend".
+
 If you want to fold two or more commits into one, replace the command
 "pick" with "squash" for the second and subsequent commit.  If the
 commits had different authors, it will attribute the squashed commit to
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 23ded48..3f6bcc0 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -340,6 +340,14 @@ do_next () {
 		pick_one $sha1 ||
 			die_with_patch $sha1 "Could not apply $sha1... $rest"
 		;;
+	amend|a)
+		comment_for_reflog amend
+
+		mark_action_done
+		pick_one $sha1 ||
+			die_with_patch $sha1 "Could not apply $sha1... $rest"
+		output git commit --amend
+		;;
 	edit|e)
 		comment_for_reflog edit
 
@@ -752,6 +760,7 @@ first and then run 'git rebase --continue' again."
 #
 # Commands:
 #  p, pick = use commit
+#  a, amend = use commit, but allow editing of the commit message
 #  e, edit = use commit, but stop for amending
 #  s, squash = use commit, but meld into previous commit
 #
diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh
index 260a231..2ca0481 100644
--- a/t/lib-rebase.sh
+++ b/t/lib-rebase.sh
@@ -9,8 +9,8 @@
 #
 #	"[<lineno1>] [<lineno2>]..."
 #
-#   If a line number is prefixed with "squash" or "edit", the respective line's
-#   command will be replaced with the specified one.
+#   If a line number is prefixed with "squash", "edit", or "amend", the
+#   respective line's command will be replaced with the specified one.
 
 set_fake_editor () {
 	echo "#!$SHELL_PATH" >fake-editor.sh
@@ -32,7 +32,7 @@ cat "$1".tmp
 action=pick
 for line in $FAKE_LINES; do
 	case $line in
-	squash|edit)
+	squash|edit|amend)
 		action="$line";;
 	*)
 		echo sed -n "${line}s/^pick/$action/p"
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 4cae019..3520480 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -470,4 +470,18 @@ test_expect_success 'avoid unnecessary reset' '
 	test 123456789 = $MTIME
 '
 
+test_expect_success 'amend' '
+	git checkout -b amend-branch master &&
+	FAKE_LINES="1 2 3 amend 4" FAKE_COMMIT_MESSAGE="E changed" git rebase -i A &&
+	git show HEAD | grep "E changed" &&
+	test $(git rev-parse master) != $(git rev-parse HEAD) &&
+	test $(git rev-parse master^) = $(git rev-parse HEAD^) &&
+	FAKE_LINES="1 2 amend 3 4" FAKE_COMMIT_MESSAGE="D changed" git rebase -i A &&
+	git show HEAD^ | grep "D changed" &&
+	FAKE_LINES="amend 1 2 3 4" FAKE_COMMIT_MESSAGE="B changed" git rebase -i A &&
+	git show HEAD~3 | grep "B changed" &&
+	FAKE_LINES="1 amend 2 3 4" FAKE_COMMIT_MESSAGE="C changed" git rebase -i A &&
+	git show HEAD~2 | grep "C changed"
+'
+
 test_done
-- 
1.6.5.rc2.18.g117a
Johannes Schindelin· Oct 4, 2009, 21:09 UTC · re: Björn Gustavsson · lore

Re: [PATCH] Teach 'rebase -i' the command "amend"

Hi,
On Sun, 4 Oct 2009, Björn Gustavsson wrote:
> Make it easier to edit just the commit message for a commit
> using 'git rebase -i' by introducing the "amend" command.

I thought that we had a discussion about this and that the consensus was that "amend" would be misleading. Maybe you can find that thread in GMane?

Ciao, Dscho

Björn Gustavsson· Oct 5, 2009, 06:08 UTC · re: Johannes Schindelin · lore

Re: [PATCH] Teach 'rebase -i' the command "amend"

2009/10/4 Johannes Schindelin <Johannes.Schindelin@gmx.de>:
> I thought that we had a discussion about this and that the consensus was
> that "amend" would be misleading.  Maybe you can find that thread in
> GMane?
I found this thread from January 2009:
http://thread.gmane.org/gmane.comp.version-control.git/105738
Having read the thread, I agree that "amend" would be misleading.

There were several suggestions for alternate command names in that thread, for example:

"msg", "msgedit", "message", "reword", "rephrase"

It think that "msgedit" was suggested by several people. ("editmsg" was also suggested, but it is not possible as the abbreviation "e" would become ambiguous.)

Would the patch have a chance to be accepted if I renamed the new command to "msgedit"?

/Björn
-- 
Björn Gustavsson, Erlang/OTP, Ericsson AB
Junio C Hamano· Oct 5, 2009, 06:33 UTC · re: Björn Gustavsson · lore

Re: [PATCH] Teach 'rebase -i' the command "amend"

Björn Gustavsson <bgustavsson@gmail.com> writes:
Show 5 quoted lines
> I found this thread from January 2009:
>
> http://thread.gmane.org/gmane.comp.version-control.git/105738
>
> Having read the thread, I agree that "amend" would be misleading.
Yeah, I thought "reword" might be a good command name back then.
Was the naming the only issue in the discussion there?
Johannes Schindelin· Oct 5, 2009, 09:17 UTC · re: Björn Gustavsson · lore

Re: [PATCH] Teach 'rebase -i' the command "amend"

Hi,
On Mon, 5 Oct 2009, Björn Gustavsson wrote:
Show 22 quoted lines
> 2009/10/4 Johannes Schindelin <Johannes.Schindelin@gmx.de>:
> > I thought that we had a discussion about this and that the consensus was
> > that "amend" would be misleading.  Maybe you can find that thread in
> > GMane?
> 
> I found this thread from January 2009:
> 
> http://thread.gmane.org/gmane.comp.version-control.git/105738
> 
> Having read the thread, I agree that "amend" would be misleading.
> 
> There were several suggestions for alternate command names
> in that thread, for example:
> 
> "msg", "msgedit", "message", "reword", "rephrase"
> 
> It think that "msgedit" was suggested by several people. ("editmsg"
> was also suggested, but it is not possible as the abbreviation "e" would
> become ambiguous.)
> 
> Would the patch have a chance to be accepted if I renamed
> the new command to "msgedit"?

My objection was that the "upcoming" (yeah, Sverre, I am imitating Duke Nukem Forever here) "merge" command would clash with "msgedit", which was why I suggested "rephrase" (but would be okay with "reword" Junio mentions).

AFAIR the naming was the only objection.

Ciao, Dscho

Sverre Rabbelier· Oct 5, 2009, 09:39 UTC · re: Johannes Schindelin · lore

Re: [PATCH] Teach 'rebase -i' the command "amend"

Heya,
2009/10/5 Johannes Schindelin <Johannes.Schindelin@gmx.de>:
> My objection was that the "upcoming" (yeah, Sverre, I am imitating Duke
> Nukem Forever here) "merge" command would clash with "msgedit", which was
> why I suggested "rephrase" (but would be okay with "reword" Junio
> mentions).

I also dislike 'msgedit' because it's abbrev-ed, I would prefer "reword" for that reason.

-- 
Cheers,

Sverre Rabbelier
Björn Gustavsson· Oct 5, 2009, 16:10 UTC · re: Sverre Rabbelier · lore

Re: [PATCH] Teach 'rebase -i' the command "amend"

On Mon, Oct 5, 2009 at 11:39 AM, Sverre Rabbelier <srabbelier@gmail.com> wrote:
Show 9 quoted lines
> 2009/10/5 Johannes Schindelin <Johannes.Schindelin@gmx.de>:
>> My objection was that the "upcoming" (yeah, Sverre, I am imitating Duke
>> Nukem Forever here) "merge" command would clash with "msgedit", which was
>> why I suggested "rephrase" (but would be okay with "reword" Junio
>> mentions).
>
> I also dislike 'msgedit' because it's abbrev-ed, I would prefer
> "reword" for that reason.
>
Thanks for the comments. "reword" it will be then. I'll send a new patch soon.
-- 
Björn Gustavsson, Erlang/OTP, Ericsson AB
Anders Melchiorsen· Oct 5, 2009, 19:43 UTC · re: Björn Gustavsson · lore

Re: [PATCH] Teach 'rebase -i' the command "amend"

Björn Gustavsson <bgustavsson@gmail.com> writes:
> Thanks for the comments. "reword" it will be then. I'll send a new patch soon.

If you could also make it possible to edit the commit summary line right in the git-rebase-todo buffer, that would be great.

Being in an editor but still not able to fix typos is a nuisance.

Cheers, Anders.

Johannes Schindelin· Oct 5, 2009, 20:50 UTC · re: Anders Melchiorsen · lore

Re: [PATCH] Teach 'rebase -i' the command "amend"

Hi,
On Mon, 5 Oct 2009, Anders Melchiorsen wrote:
Show 8 quoted lines
> Björn Gustavsson <bgustavsson@gmail.com> writes:
> 
> > Thanks for the comments. "reword" it will be then. I'll send a new patch soon.
> 
> If you could also make it possible to edit the commit summary line
> right in the git-rebase-todo buffer, that would be great.
> 
> Being in an editor but still not able to fix typos is a nuisance.
NAK.

Supporting that would be totally out of line with the way rebase -i is supposed to work.

Besides, if you already have typos in the commit subject, you _better_ check the whole commit message, so: double NAK.

Ciao, Dscho

Junio C Hamano· Oct 5, 2009, 21:07 UTC · re: Johannes Schindelin · lore

Re: [PATCH] Teach 'rebase -i' the command "amend"

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
Show 6 quoted lines
>> Being in an editor but still not able to fix typos is a nuisance.
>
> NAK.
>
> Supporting that would be totally out of line with the way rebase -i is 
> supposed to work.

If the rebase insn sheet were richer, and had a way to show the full message, like this:

pick 4973aa2 git-pull: dead code removal
    Back when a74b170 (git-pull: disallow implicit merging to detached HEAD,
    2007-01-15) added this check, $? referred to the error status of reading
    HEAD as a symbolic-ref; but cd67e4d (Teach 'git pull' about --rebase,
    2007-11-28) moved the command away from where the check is, and nobody
    noticed the breakage.  Ever since, $? has always been 0 (tr at the end of
    the pipe to find merge_head never fails) and other case arms were never
    reached.
    
    These days, error_on_no_merge_candidates function is prepared to handle a
    detached HEAD case, which was what the code this patch removes used to
    handle.
    
    Signed-off-by: Junio C Hamano <gitster@pobox.com>

I do not see why we shouldn't allow people to edit any part of the above to reword.

I would even understand (but not necessarily agree) if somebody wants to give the patch text and let users edit to reapply.

So I do not agree with your "totally out of line" at all.
> Besides, if you already have typos in the commit subject, you _better_ 
> check the whole commit message, so: double NAK.
That sounds a bit too dogmatic.

But I tend to agree with you that we would be better off not accepting such a "retitle" patch, as it strongly encourages single-liner commit log messages.

Oh, there was no patch?  Then nevermind...
Sverre Rabbelier· Oct 5, 2009, 21:13 UTC · re: Junio C Hamano · lore

Re: [PATCH] Teach 'rebase -i' the command "amend"

Heya,
2009/10/5 Junio C Hamano <gitster@pobox.com>:
> If the rebase insn sheet were richer, and had a way to show the full
> message, like this:

But that's not what rebase -i's insn sheet is about is it? It's not "rewrite my commits so that they are the way I want them", it's about "change the order of my commits"/squash some/drop some. The polishing of the commits itself is done after finishing the insn sheet.

> I do not see why we shouldn't allow people to edit any part of the above
> to reword.

Because it then becomes very hard to do any actual reordering. I'm not saying it's a bad idea, just that it's a bad idea to do it in 'git rebase -i', conceptually. Although what you suggest would be useful to me, I just think it should be a different command, git rewrite perhaps :P. Definitely not "git rebase -i --rewrite-message" though, becaue it is not at all about rebasing anymore.

-- 
Cheers,

Sverre Rabbelier
Johannes Schindelin· Oct 5, 2009, 21:26 UTC · re: Junio C Hamano · lore

Re: [PATCH] Teach 'rebase -i' the command "amend"

Hi,
On Mon, 5 Oct 2009, Junio C Hamano wrote:
Show 34 quoted lines
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> >> Being in an editor but still not able to fix typos is a nuisance.
> >
> > NAK.
> >
> > Supporting that would be totally out of line with the way rebase -i is 
> > supposed to work.
> 
> If the rebase insn sheet were richer, and had a way to show the full
> message, like this:
> 
> pick 4973aa2 git-pull: dead code removal
>     Back when a74b170 (git-pull: disallow implicit merging to detached HEAD,
>     2007-01-15) added this check, $? referred to the error status of reading
>     HEAD as a symbolic-ref; but cd67e4d (Teach 'git pull' about --rebase,
>     2007-11-28) moved the command away from where the check is, and nobody
>     noticed the breakage.  Ever since, $? has always been 0 (tr at the end of
>     the pipe to find merge_head never fails) and other case arms were never
>     reached.
>     
>     These days, error_on_no_merge_candidates function is prepared to handle a
>     detached HEAD case, which was what the code this patch removes used to
>     handle.
>     
>     Signed-off-by: Junio C Hamano <gitster@pobox.com>
> 
> I do not see why we shouldn't allow people to edit any part of the above
> to reword.
> 
> I would even understand (but not necessarily agree) if somebody wants to
> give the patch text and let users edit to reapply.
> 
> So I do not agree with your "totally out of line" at all.

Are you serious? "rebase -i" was _always_ about showing an edit script, i.e. to tell Git _what_ you want to do with _which_ commits, identified by short commit names.

The oneline was _always_ meant as a pure convenience for the user.

This paradigm has been true even to back when "rebase -i" was still called "edit-patch-series", and that is the reason I claimed that it is totally out of line. Because it is.

Show 8 quoted lines
> > Besides, if you already have typos in the commit subject, you _better_ 
> > check the whole commit message, so: double NAK.
> 
> That sounds a bit too dogmatic.
> 
> But I tend to agree with you that we would be better off not accepting
> such a "retitle" patch, as it strongly encourages single-liner commit log
> messages.
Now, that is at least as dogmatic as what I proposed.
> Oh, there was no patch?  Then nevermind...

Sorry, but I changed my mind on this attitude. Earlier, you would find me valuing arguments only when backed up by code. But since the GitTogether in Berlin I know of at least one discussion where somebody simply had not enough time to back up a (submodule-related) argument, the validity of the argument notwithstanding.

So not always is a "no patch? Nevermind" the correct thing to do.

In this particular case, however, I agree. Just touching up the first line of commit messages is such a special case, and so removed from what a "rebase" is about that I would claim that this case would be better handled by a really trivial shell script that launches an editor and then drives filter-branch based on what the user said.

Ciao, Dscho

Johannes Sixt· Oct 6, 2009, 05:53 UTC · re: Johannes Schindelin · lore

Re: [PATCH] Teach 'rebase -i' the command "amend"

Johannes Schindelin schrieb:
Show 5 quoted lines
> Are you serious?  "rebase -i" was _always_ about showing an edit script, 
> i.e. to tell Git _what_ you want to do with _which_ commits, identified by 
> short commit names.
> 
> The oneline was _always_ meant as a pure convenience for the user.

Good that you mention it: The one-liner is really convenient. I tend to replace it by reminders like "====== fix unused var", "===== edit msg", etc. (after marking the commit "edit") and I'm glad that rebase-i later prints the one-liner from the insn file rather than the original subject line before it stops.

(I do it this way because on Windows I can't afford to call rebase-i on a long patch series for every single task. Rather, I plan the tasks while the insn editor is open, and this way I keep reminders about what the plan was.)

No, I definitely don't want the one-liners to end up in the commit message. ;-)

-- Hannes

← back to recent threads