threads / patch / 11545

patchadditional help when editing during interactive rebase

Subject: [PATCH] additional help when editing during interactive rebase

## tl;dr

6 messages between Jan 9, 2008 and Jan 11, 2008. Diffs are folded; open one to read it.

replies: 5people: 3as markdown or json

William Morgan· Jan 9, 2008, 02:32 UTC · lore

I personally would have found this message useful the first time I used git rebase --interactive. YMMV.

Signed-off-by: William Morgan <wmorgan-git@masanjin.net>
---
 git-rebase--interactive.sh |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)
Show changes to git-rebase--interactive.sh +4 −0
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index acdcc54..d53d283 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -263,6 +263,10 @@ do_next () {
 		warn
 		warn "	git commit --amend"
 		warn
+		warn "Once amended, continue with"
+		warn
+		warn "	git rebase --continue"
+		warn
 		exit 0
 		;;
 	squash|s)
-- 
1.5.4.rc2.68.ge708a-dirty


-- 
William <wmorgan-git@masanjin.net>
Junio C Hamano· Jan 9, 2008, 02:55 UTC · re: William Morgan · lore

Re: [PATCH] additional help when editing during interactive rebase

William Morgan <wmorgan-git@masanjin.net> writes:
> I personally would have found this message useful the first time I used
> git rebase --interactive. YMMV.

Aside from this message being inappropriate as a proposed commit log message, I think what the patch tries to achieve is a worthy UI improvement.

I would have removed those empty lines around the instruction if I were patching this, though. Losing 5 lines out of 25-line terminal was marginally Ok. Losing 9 lines 4 lines too many and is unacceptable.

Thoughts?
Show 26 quoted lines
> Signed-off-by: William Morgan <wmorgan-git@masanjin.net>
> ---
>  git-rebase--interactive.sh |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
>
> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index acdcc54..d53d283 100755
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -263,6 +263,10 @@ do_next () {
>  		warn
>  		warn "	git commit --amend"
>  		warn
> +		warn "Once amended, continue with"
> +		warn
> +		warn "	git rebase --continue"
> +		warn
>  		exit 0
>  		;;
>  	squash|s)
> -- 
> 1.5.4.rc2.68.ge708a-dirty
>
>
> -- 
> William <wmorgan-git@masanjin.net>
William Morgan· Jan 9, 2008, 03:29 UTC · re: Junio C Hamano · lore

Let the user know how to continue a rebase after amending a commit during a git rebase --interactive session.

Signed-off-by: William Morgan <wmorgan@masanjin.net>
---
 git-rebase--interactive.sh |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)
Show changes to git-rebase--interactive.sh +2 −3
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index acdcc54..ccef1ac 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -258,11 +258,10 @@ do_next () {
 			die_with_patch $sha1 "Could not apply $sha1... $rest"
 		make_patch $sha1
 		: > "$DOTEST"/amend
-		warn
 		warn "You can amend the commit now, with"
-		warn
 		warn "	git commit --amend"
-		warn
+		warn "Once amended, continue with"
+		warn "	git rebase --continue"
 		exit 0
 		;;
 	squash|s)
-- 
1.5.4.rc2.69.g10f0

-- 
William <wmorgan-git@masanjin.net>
Johannes Schindelin· Jan 9, 2008, 11:23 UTC · re: Junio C Hamano · lore

Re: [PATCH] additional help when editing during interactive rebase

Hi,
On Tue, 8 Jan 2008, Junio C Hamano wrote:
Show 5 quoted lines
> I would have removed those empty lines around the instruction if I were 
> patching this, though.  Losing 5 lines out of 25-line terminal was 
> marginally Ok.  Losing 9 lines 4 lines too many and is unacceptable.
> 
> Thoughts?

I wonder if it would not make even more sense to record the current HEAD name, and call "commit --amend" if it is the same upon "--continue".

Note that "commit --amend" is _already_ called automatically if the index is dirty (but agrees with the working directory).

Then the user would be spared some additional typing, and the help could be changed to hint at "rebase --continue". It also would make things more consistent.

Ciao, Dscho

Junio C Hamano· Jan 11, 2008, 08:42 UTC · re: Johannes Schindelin · lore

Re: [PATCH] additional help when editing during interactive rebase

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
Show 12 quoted lines
> Hi,
>
> On Tue, 8 Jan 2008, Junio C Hamano wrote:
>
>> I would have removed those empty lines around the instruction if I were 
>> patching this, though.  Losing 5 lines out of 25-line terminal was 
>> marginally Ok.  Losing 9 lines 4 lines too many and is unacceptable.
>> 
>> Thoughts?
>
> I wonder if it would not make even more sense to record the current HEAD 
> name, and call "commit --amend" if it is the same upon "--continue".

My understanding of the original issue is that "git-rebase -i" stops at 'edit' and gives the user a chance to muck with the commit, saying "do whatever you want now and then record the result with git commit --amend". The user can follow that but then needs to say "git rebase --continue" after that. The insn does not talk about it, so after running "git commit --amend" as told, a clueless user is left wondering "huh, and then now what?".

Do you mean you would instead suggest "git rebase --continue" in the insn, and make the workflow like this:

	$ git rebase -i ...
        Now do whatever you want and say "rebase --continue"
	$ edit foo.c
        $ git add foo.c
        $ git rebase --continue

and have "rebase --continue" to continue with the modified contents recorded in the index, invoking "git commit --amend", but doing so only if the user hasn't run "git commit" with or without --amend yet?

It feels like a better automation than what we currently have, but I somewhat worry how that would change the user experience for using 'edit' to split a commit into two or more.

Johannes Schindelin· Jan 11, 2008, 11:29 UTC · re: Junio C Hamano · lore

Re: [PATCH] additional help when editing during interactive rebase

Hi,
On Fri, 11 Jan 2008, Junio C Hamano wrote:
Show 13 quoted lines
> Do you mean you would instead suggest "git rebase --continue" in
> the insn, and make the workflow like this:
> 
> 	$ git rebase -i ...
>         Now do whatever you want and say "rebase --continue"
> 	$ edit foo.c
>         $ git add foo.c
>         $ git rebase --continue
> 
> and have "rebase --continue" to continue with the modified
> contents recorded in the index, invoking "git commit --amend",
> but doing so only if the user hasn't run "git commit" with or
> without --amend yet?
Yes, exactly.
> It feels like a better automation than what we currently have,
> but I somewhat worry how that would change the user experience
> for using 'edit' to split a commit into two or more.

If you want to split a commit into two or more, you will already have committed twice when you say "--continue", and all is fine.

However, if you do the first commit, and then only add the files for the second commit, the HEAD's commit name has changed! And so, rebase can pick up on that, and avoid the --amend.

IOW something like below. However, this patch does not yet make "rebase -i" call "commit --amend" automatically when both the index and HEAD are unchanged.

-- snipsnap -- [PATCH] rebase -i: only ever commit --amend when HEAD is untouched

When a commit is marked to edit, and the index is dirty when "rebase --continue" is called, that state will be committed with the "--amend" option.

However, this is wrong when the user wanted to split the commit.

Luckily, we can pick up on that, by recording the HEAD's name in the file "amend", and only --amend when no commit was made in the interim.

Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
---
 git-rebase--interactive.sh |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)
Show changes to git-rebase--interactive.sh +4 −2
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index acdcc54..4a8a980 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -257,7 +257,7 @@ do_next () {
 		pick_one $sha1 ||
 			die_with_patch $sha1 "Could not apply $sha1... $rest"
 		make_patch $sha1
-		: > "$DOTEST"/amend
+		git rev-parse HEAD > "$DOTEST"/amend
 		warn
 		warn "You can amend the commit now, with"
 		warn
@@ -378,7 +378,9 @@ do
 		else
 			. "$DOTEST"/author-script ||
 				die "Cannot find the author identity"
-			if test -f "$DOTEST"/amend
+			if test -f "$DOTEST"/amend &&
+				test $(git rev-parse HEAD) = \
+					$(cat "$DOTEST"/amend)
 			then
 				git reset --soft HEAD^ ||
 				die "Cannot rewind the HEAD"

← back to recent threads