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

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

[Junio C Hamano]

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.

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

[Johannes Schindelin]

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

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

[Johannes Schindelin]

---

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

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

[Junio C Hamano]

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.

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

[Johannes Schindelin]

Hi,

On Thu, 22 Nov 2007, Junio C Hamano wrote:

> 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

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

[Jeff King]

On Thu, Nov 22, 2007 at 12:30:10PM +0000, Johannes Schindelin wrote:

> 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

[PATCH] rebase -i: give rerere a chance

[Johannes Schindelin]

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

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

[Junio C Hamano]

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

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

[Johannes Schindelin]

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?

Ciao,
Dscho

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

[David Kastrup]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> 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

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

[David Brown]

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

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

[Johannes Schindelin]

Hi,

On Thu, 22 Nov 2007, David Brown wrote:

> 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

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

[Andy Parkins]

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

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

[Andy Parkins]

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