Skip to contentSkip to conversation

Teach 'rebase -i' the command "amend"

PATCH NAK Björn Gustavsson 6 participants 13 messages Jun 15, 2016 Documentation

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Teach 'rebase -i' the command "amend"

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

---
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(-)
Documentation/git-rebase.txt +3 -0
index 0aefc34..28b90ec 100644
@@ -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
git-rebase--interactive.sh +9 -0
index 23ded48..3f6bcc0 100755
@@ -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
 #
t/lib-rebase.sh +3 -3
index 260a231..2ca0481 100644
@@ -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"
t/t3404-rebase-interactive.sh +14 -0
index 4cae019..3520480 100755
@@ -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
Signature · 1 line
1.6.5.rc2.18.g117a
#1 lore raw

Hi,

Quoted 2 lines · Björn Gustavsson

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

#2 lore raw

2009/10/4 Johannes Schindelin <[email protected]>:

Quoted 3 lines

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

Signature · 1 line
Björn Gustavsson, Erlang/OTP, Ericsson AB
#4 lore raw
In reply to Björn Gustavsson

Hi,

Quoted 14 lines · Björn Gustavsson

2009/10/4 Johannes Schindelin <[email protected]>:

Quoted 3 lines

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:

Quoted 8 lines

"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

#5 lore raw

Heya,

2009/10/5 Johannes Schindelin <[email protected]>:

Quoted 4 lines

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.

Signature · 3 lines
Cheers,

Sverre Rabbelier
#6 lore raw
Quoted 9 lines · Sverre Rabbelier

2009/10/5 Johannes Schindelin <[email protected]>:

Quoted 4 lines

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.

Signature · 1 line
Björn Gustavsson, Erlang/OTP, Ericsson AB
#7 lore raw
Quoted 1 line · Björn Gustavsson

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.

#8 lore raw

Hi,

Quoted 8 lines · Anders Melchiorsen
Quoted 1 line · Björn Gustavsson

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

#9 lore raw
Quoted 6 lines · Johannes Schindelin
Quoted 1 line

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.

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.

Quoted 2 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.

Oh, there was no patch? Then nevermind...

#10 lore raw

Hi,

Quoted 27 lines · Junio C Hamano
Quoted 6 lines · Johannes Schindelin
Quoted 1 line

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.

Quoted 7 lines

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.

Quoted 8 lines
Quoted 2 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.

Quoted 1 line

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

#11 lore raw
Quoted 5 lines · Johannes Schindelin

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

#12 lore raw
In reply to Junio C Hamano

Heya,

2009/10/5 Junio C Hamano <[email protected]>:

Quoted 2 lines

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.

Quoted 2 lines

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.

Signature · 3 lines
Cheers,

Sverre Rabbelier