threads / discuss / 10975

Reordering lines in "git-rebase -i" task sheet

Subject: Reordering lines in "git-rebase -i" task sheet

## tl;dr

14 messages between Nov 22, 2007 and Nov 29, 2007.

replies: 13people: 6as markdown or json

Junio C Hamano· Nov 22, 2007, 09:21 UTC · lore

When you start "git rebase -i" to reorder a few commits, you are presented with an editor buffer with 9 or so lines of help text and then lines you are supposed to edit, but most editors start with your edit cursor at line 1. IOW, _everybody_ has to move their cursor down at least 9 lines before doing anything meaningful (or they can remove 9 lines but that is the same thing).

Could we please reorder the lines so that list of initial "pick"s come first and have the help comment lines at the very end?

That is the reason why git commit leaves the first line empty in its initial template. It is to allow the user to immediately start typing.

Johannes Schindelin· Nov 22, 2007, 11:15 UTC · re: Junio C Hamano · lore

[PATCH] rebase -i: move help to end of todo file

Many editors start in the first line, so the 9-line help text was an annoyance. So move it to the end.

Requested by Junio.
While at it, add a hint how to abort the rebase.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
	On Thu, 22 Nov 2007, Junio C Hamano wrote:
	> Could we please reorder the lines so that list of initial 
	> "pick"s come first and have the help comment lines at the very 
	> end?
	How about this?  I am hesitant to remove _everything_, since quite 
	a few people seem to be allergic to man pages, so they fire up 
	rebase -i without any clue.
 git-rebase--interactive.sh |   15 +++++++++------
 1 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index a6dc72a..aca2fbb 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -480,20 +480,23 @@ do
 		SHORTHEAD=$(git rev-parse --short $HEAD)
 		SHORTONTO=$(git rev-parse --short $ONTO)
 		cat > "$TODO" << EOF
-# Rebasing $SHORTUPSTREAM..$SHORTHEAD onto $SHORTONTO
-#
+# Rebasing $SHORTUPSTREAM..$SHORTHEAD onto $SHORTONTO (see end for help)
+EOF
+		git rev-list $MERGES_OPTION --pretty=oneline --abbrev-commit \
+			--abbrev=7 --reverse --left-right --cherry-pick \
+			$UPSTREAM...$HEAD | \
+			sed -n "s/^>/pick /p" >> "$TODO"
+		cat >> "$TODO" << EOF
+
 # Commands:
 #  pick = use commit
 #  edit = use commit, but stop for amending
 #  squash = use commit, but meld into previous commit
 #
 # If you remove a line here THAT COMMIT WILL BE LOST.
+# However, if you remove everything, the rebase will be aborted.
 #
 EOF
-		git rev-list $MERGES_OPTION --pretty=oneline --abbrev-commit \
-			--abbrev=7 --reverse --left-right --cherry-pick \
-			$UPSTREAM...$HEAD | \
-			sed -n "s/^>/pick /p" >> "$TODO"
 
 		has_action "$TODO" ||
 			die_abort "Nothing to do"

-- 
1.5.3.6.1977.g54d30
Johannes Schindelin· Nov 22, 2007, 11:41 UTC · re: Johannes Schindelin · lore

[PATCH] Adjust t3404 to ignore empty lines in the todo file

---
	... and this fixes the tests again.  I changed the single "#"
	line after the commits to an empty line, since it made things
	more readable to this developer.  The test expected "#", though.
	Please amend.
 t/t3404-rebase-interactive.sh |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index f1039d1..907c7f9 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -149,7 +149,8 @@ test_expect_success 'stop on conflicting pick' '
 	diff -u expect .git/.dotest-merge/patch &&
 	diff -u expect2 file1 &&
 	test 4 = $(grep -v "^#" < .git/.dotest-merge/done | wc -l) &&
-	test 0 = $(grep -v "^#" < .git/.dotest-merge/git-rebase-todo | wc -l)
+	test 0 = $(grep -ve "^#" -e "^$" < .git/.dotest-merge/git-rebase-todo |
+		wc -l)
 '
 
 test_expect_success 'abort' '

-- 
1.5.3.6.1977.g54d30
Junio C Hamano· Nov 22, 2007, 11:46 UTC · re: Johannes Schindelin · lore

Re: [PATCH] rebase -i: move help to end of todo file

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 	How about this?  I am hesitant to remove _everything_, since quite 
> 	a few people seem to be allergic to man pages, so they fire up 
> 	rebase -i without any clue.

Oh, I wouldn't dream of suggesting complete removal of the help text, but leaving the single line at the beginning is not an improvement. What's on that single line is not particularly useful but that is a separate issue.

Moving everything down will hurt ONLY when (1) the rebase is about a large series (more than 24 commits in vt100) AND (2) the user hasn't run "rebase -i" before and does not know that there is a reminder insn at the end. Now is it likely for a newbie to run "rebase -i" with 20-30 commits and that invocation is his first "rebase -i" invocation in his life?

The new help line at the end is helpful, by the way. I always had "Huh?" moment, and did ^Z followed by kill %% instead.

Johannes Schindelin· Nov 22, 2007, 12:30 UTC · re: Junio C Hamano · lore

Re: [PATCH] rebase -i: move help to end of todo file

Hi,
On Thu, 22 Nov 2007, Junio C Hamano wrote:
Show 8 quoted lines
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > 	How about this?  I am hesitant to remove _everything_, since quite 
> > 	a few people seem to be allergic to man pages, so they fire up 
> > 	rebase -i without any clue.
> 
> Oh, I wouldn't dream of suggesting complete removal of the help
> text, 
I meant to "move" everything.  Sorry.
> but leaving the single line at the beginning is not an improvement.  
> What's on that single line is not particularly useful but that is a 
> separate issue.
Okay, so you'd like this better?

-- snipsnap -- [PATCH] rebase -i: move help to end of todo file

Many editors start in the first line, so the 9-line help text was an annoyance. So move it to the end.

Requested by Junio.
While at it, add a hint how to abort the rebase.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 git-rebase--interactive.sh    |   14 ++++++++------
 t/t3404-rebase-interactive.sh |    3 ++-
 2 files changed, 10 insertions(+), 7 deletions(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index a6dc72a..58fde89 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -479,8 +479,13 @@ do
 		SHORTUPSTREAM=$(git rev-parse --short $UPSTREAM)
 		SHORTHEAD=$(git rev-parse --short $HEAD)
 		SHORTONTO=$(git rev-parse --short $ONTO)
-		cat > "$TODO" << EOF
-# Rebasing $SHORTUPSTREAM..$SHORTHEAD onto $SHORTONTO
+		git rev-list $MERGES_OPTION --pretty=oneline --abbrev-commit \
+			--abbrev=7 --reverse --left-right --cherry-pick \
+			$UPSTREAM...$HEAD | \
+			sed -n "s/^>/pick /p" > "$TODO"
+		cat >> "$TODO" << EOF
+
+# Rebase $SHORTUPSTREAM..$SHORTHEAD onto $SHORTONTO
 #
 # Commands:
 #  pick = use commit
@@ -488,12 +493,9 @@ do
 #  squash = use commit, but meld into previous commit
 #
 # If you remove a line here THAT COMMIT WILL BE LOST.
+# However, if you remove everything, the rebase will be aborted.
 #
 EOF
-		git rev-list $MERGES_OPTION --pretty=oneline --abbrev-commit \
-			--abbrev=7 --reverse --left-right --cherry-pick \
-			$UPSTREAM...$HEAD | \
-			sed -n "s/^>/pick /p" >> "$TODO"
 
 		has_action "$TODO" ||
 			die_abort "Nothing to do"
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index f1039d1..907c7f9 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -149,7 +149,8 @@ test_expect_success 'stop on conflicting pick' '
 	diff -u expect .git/.dotest-merge/patch &&
 	diff -u expect2 file1 &&
 	test 4 = $(grep -v "^#" < .git/.dotest-merge/done | wc -l) &&
-	test 0 = $(grep -v "^#" < .git/.dotest-merge/git-rebase-todo | wc -l)
+	test 0 = $(grep -ve "^#" -e "^$" < .git/.dotest-merge/git-rebase-todo |
+		wc -l)
 '
 
 test_expect_success 'abort' '

-- 
1.5.3.6.1977.g54d30
Jeff King· Nov 22, 2007, 12:31 UTC · re: Johannes Schindelin · lore

Re: [PATCH] rebase -i: move help to end of todo file

On Thu, Nov 22, 2007 at 12:30:10PM +0000, Johannes Schindelin wrote:
Show 7 quoted lines
> I meant to "move" everything.  Sorry.
> 
> > but leaving the single line at the beginning is not an improvement.  
> > What's on that single line is not particularly useful but that is a 
> > separate issue.
> 
> Okay, so you'd like this better?

I like it much better (I was about to send the same complaint about the top line, but Junio beat me to it).

-Peff
Johannes Schindelin· Nov 22, 2007, 11:18 UTC · re: Junio C Hamano · lore

[PATCH] rebase -i: give rerere a chance

Like non-interactive rebase, interactive mode now calls rerere when appropriate.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
	While I was working with rebase -i yesterday, I realised that it
	did not call rerere at all.  And sure enough, there was a
	NEEDSWORK in rebase -i for that.
	Now, I _hope_ I got all the locations where the rerere should be
	cleared, but this could use a few pairs of eyeballs.
 git-rebase--interactive.sh |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index ead7da7..a6dc72a 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -81,6 +81,7 @@ make_patch () {
 
 die_with_patch () {
 	make_patch "$1"
+	git rerere
 	die "$2"
 }
 
@@ -166,13 +167,13 @@ pick_one_preserving_merges () {
 			msg="$(git cat-file commit $sha1 | sed -e '1,/^$/d')"
 			# No point in merging the first parent, that's HEAD
 			new_parents=${new_parents# $first_parent}
-			# NEEDSWORK: give rerere a chance
 			if ! GIT_AUTHOR_NAME="$GIT_AUTHOR_NAME" \
 				GIT_AUTHOR_EMAIL="$GIT_AUTHOR_EMAIL" \
 				GIT_AUTHOR_DATE="$GIT_AUTHOR_DATE" \
 				output git merge $STRATEGY -m "$msg" \
 					$new_parents
 			then
+				git rerere
 				printf "%s\n" "$msg" > "$GIT_DIR"/MERGE_MSG
 				die Error redoing merge $sha1
 			fi
@@ -360,6 +361,7 @@ do
 	--abort)
 		comment_for_reflog abort
 
+		git rerere clear
 		test -d "$DOTEST" || die "No interactive rebase running"
 
 		HEADNAME=$(cat "$DOTEST"/head-name)
@@ -376,6 +378,7 @@ do
 	--skip)
 		comment_for_reflog skip
 
+		git rerere clear
 		test -d "$DOTEST" || die "No interactive rebase running"
 
 		output git reset --hard && do_rest

-- 
1.5.3.6.1977.g54d30
Junio C Hamano· Nov 29, 2007, 00:17 UTC · re: Johannes Schindelin · lore

Re: [PATCH] rebase -i: give rerere a chance

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
Show 12 quoted lines
> @@ -166,13 +167,13 @@ pick_one_preserving_merges () {
>  			msg="$(git cat-file commit $sha1 | sed -e '1,/^$/d')"
>  			# No point in merging the first parent, that's HEAD
>  			new_parents=${new_parents# $first_parent}
> -			# NEEDSWORK: give rerere a chance
>  			if ! GIT_AUTHOR_NAME="$GIT_AUTHOR_NAME" \
>  				GIT_AUTHOR_EMAIL="$GIT_AUTHOR_EMAIL" \
>  				GIT_AUTHOR_DATE="$GIT_AUTHOR_DATE" \
>  				output git merge $STRATEGY -m "$msg" \
>  					$new_parents
>  			then
> +				git rerere

This comment is not about this rerere change, but output is a shell function and I vaguely recall we had a discussion on "VAR=VAL cmd" form of single-shot export not working for them as expected...

Johannes Schindelin· Nov 29, 2007, 00:22 UTC · re: Junio C Hamano · lore

Re: [PATCH] rebase -i: give rerere a chance

Hi,
On Wed, 28 Nov 2007, Junio C Hamano wrote:
Show 18 quoted lines
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > @@ -166,13 +167,13 @@ pick_one_preserving_merges () {
> >  			msg="$(git cat-file commit $sha1 | sed -e '1,/^$/d')"
> >  			# No point in merging the first parent, that's HEAD
> >  			new_parents=${new_parents# $first_parent}
> > -			# NEEDSWORK: give rerere a chance
> >  			if ! GIT_AUTHOR_NAME="$GIT_AUTHOR_NAME" \
> >  				GIT_AUTHOR_EMAIL="$GIT_AUTHOR_EMAIL" \
> >  				GIT_AUTHOR_DATE="$GIT_AUTHOR_DATE" \
> >  				output git merge $STRATEGY -m "$msg" \
> >  					$new_parents
> >  			then
> > +				git rerere
> 
> This comment is not about this rerere change, but output is a shell
> function and I vaguely recall we had a discussion on "VAR=VAL cmd" form
> of single-shot export not working for them as expected...

Hmm. What do you propose? In the long run, I _want_ to have rebase as a builtin, which would solve this problem, probably. But in the short run?

Ciao, Dscho

David Kastrup· Nov 29, 2007, 09:59 UTC · re: Johannes Schindelin · lore

Re: [PATCH] rebase -i: give rerere a chance

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
Show 25 quoted lines
> Hi,
>
> On Wed, 28 Nov 2007, Junio C Hamano wrote:
>
>> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>> 
>> > @@ -166,13 +167,13 @@ pick_one_preserving_merges () {
>> >  			msg="$(git cat-file commit $sha1 | sed -e '1,/^$/d')"
>> >  			# No point in merging the first parent, that's HEAD
>> >  			new_parents=${new_parents# $first_parent}
>> > -			# NEEDSWORK: give rerere a chance
>> >  			if ! GIT_AUTHOR_NAME="$GIT_AUTHOR_NAME" \
>> >  				GIT_AUTHOR_EMAIL="$GIT_AUTHOR_EMAIL" \
>> >  				GIT_AUTHOR_DATE="$GIT_AUTHOR_DATE" \
>> >  				output git merge $STRATEGY -m "$msg" \
>> >  					$new_parents
>> >  			then
>> > +				git rerere
>> 
>> This comment is not about this rerere change, but output is a shell
>> function and I vaguely recall we had a discussion on "VAR=VAL cmd" form
>> of single-shot export not working for them as expected...
>
> Hmm.  What do you propose?  In the long run, I _want_ to have rebase as a 
> builtin, which would solve this problem, probably.  But in the short run?
Well, something like
if ! ( GIT_xxx=A; export GIT_xxx; output git merge ... )
should likely work.
-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum
David Brown· Nov 22, 2007, 19:25 UTC · re: Junio C Hamano · lore

Re: Reordering lines in "git-rebase -i" task sheet

On Thu, Nov 22, 2007 at 01:21:55AM -0800, Junio C Hamano wrote:
>That is the reason why git commit leaves the first line empty in
>its initial template.  It is to allow the user to immediately
>start typing.

Unless of course your editor tries to be "helpful" and remember what line you were on previously in a file.

David
Johannes Schindelin· Nov 22, 2007, 22:11 UTC · re: David Brown · lore

Re: Reordering lines in "git-rebase -i" task sheet

Hi,
On Thu, 22 Nov 2007, David Brown wrote:
Show 8 quoted lines
> On Thu, Nov 22, 2007 at 01:21:55AM -0800, Junio C Hamano wrote:
> 
> > That is the reason why git commit leaves the first line empty in its 
> > initial template.  It is to allow the user to immediately start 
> > typing.
> 
> Unless of course your editor tries to be "helpful" and remember what 
> line you were on previously in a file.

IMHO this objection is irrelevant, since these helpful editors do not have any preference as to where the help text should be.

Hth, Dscho

Andy Parkins· Nov 23, 2007, 08:54 UTC · re: Junio C Hamano · lore

Re: Reordering lines in "git-rebase -i" task sheet

Junio C Hamano wrote:
> Could we please reorder the lines so that list of initial
> "pick"s come first and have the help comment lines at the very
> end?
I'm not convinced that this is a great idea.

Mostly with git-rebase -i one is reordering the most recent commits, which are at the bottom. Personally, I am often reordering commits that are as yet unpushed, so for ease-of-thinking I type

 git rebase -i origin/master

again and again. Hence the early lines are probably already sorted into a nice order and it's commits at the bottom of the list that need a bit of massaging.

At present it's easy to get to them because a shift-G or ctrl-end, or $END_OF_FILE takes you there. With the above change it's now more steps to move to the interesting part.

Now if you were suggesting flipping the sort direction at the same time, I could get on board - but that is perhaps one confusing step too far.

> That is the reason why git commit leaves the first line empty in
> its initial template.  It is to allow the user to immediately
> start typing.

That's different, for a commit message, the first line is exactly where one would want to be.

Not a strong objection.
Andy
-- 
Dr Andy Parkins, M Eng (hons), MIET
andyparkins@gmail.com
Andy Parkins· Nov 26, 2007, 13:14 UTC · re: Andy Parkins · lore

Re: Reordering lines in "git-rebase -i" task sheet

Andy Parkins wrote:
> I'm not convinced that this is a great idea.
I've just tried it and (for me) I don't like it.
 - It's now harder to find the commits I want to edit, the cursor still
   doesn't start in the right place, but now I can't get to the right place
   quickly (the right place being the bottom of the list) because the
   interesting bits are neither at the top nor at the bottom
 - vim no longer automatically loads a reasonable syntax highlighting mode
   making the comment lines appear the same as the pick lines.
 - On a long rebase list the important message explaining what I'm looking
   at isn't on screen.
Andy
-- 
Dr Andy Parkins, M Eng (hons), MIET
andyparkins@gmail.com

← back to recent threads