[RFC] Add "edit" action for interactive rebase?

[RFC] Add "edit" action for interactive rebase?

[Andrew Wong]

Occasionally, while I'm in the middle of an interactive rebase, I'd change my
mind about the todo list and want to modify it.  This means manually digging
out the todo file from the rebase directory, and invoking the editor.  So I
thought it might be convenient to have an "edit" action that simply invokes the
editor on the todo file but do nothing else.

This should be safe to do in the middle of a rebase, since we don't preprocess
the todo file and generate any state from it.  I've also been manually editing
the todo file a while now, and I never ran into any issues.

I wonder if any others have ever ran into this situation, and would this be
a useful feature to have in interactive rebase? Comments?

This patch doesn't have any documentations yet. I'll add some documentations in
another patch if we decide to include this.

Andrew Wong (1):
  rebase -i: Teach "--edit" action

 git-rebase--interactive.sh |  6 ++++++
 git-rebase.sh              | 14 ++++++++++++++
 2 files changed, 20 insertions(+)

-- 
1.7.12.289.g0ce9864.dirty

[PATCH] rebase -i: Teach "--edit" action

[Andrew Wong]

This allows users to edit the todo list while they're in the middle of
an interactive rebase.

Signed-off-by: Andrew Wong <andrew.kw.w@gmail.com>
---
 git-rebase--interactive.sh |  6 ++++++
 git-rebase.sh              | 14 ++++++++++++++
 2 files changed, 20 insertions(+)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index a09e842..e9dbcf3 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -775,6 +775,12 @@ skip)
 
 	do_rest
 	;;
+edit)
+  git_sequence_editor "$todo" ||
+    die_abort "Could not execute editor"
+
+  exit
+  ;;
 esac
 
 git var GIT_COMMITTER_IDENT >/dev/null ||
diff --git a/git-rebase.sh b/git-rebase.sh
index 15da926..c394b8d 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -38,6 +38,7 @@ C=!                passed to 'git apply'
 continue!          continue
 abort!             abort and check out the original branch
 skip!              skip current patch and continue
+edit!              edit the todo list during interactive rebase
 "
 . git-sh-setup
 . git-sh-i18n
@@ -194,6 +195,10 @@ do
 		test $total_argc -eq 2 || usage
 		action=${1##--}
 		;;
+	--edit)
+		test $total_argc -eq 2 || usage
+		action=${1##--}
+		;;
 	--onto)
 		test 2 -le "$#" || usage
 		onto="$2"
@@ -306,6 +311,12 @@ then
 	fi
 fi
 
+if test "$action" = "edit" &&
+  test "$type" != "interactive"
+then
+  die "$(gettext "The --edit action can only be used during interactive rebase.")"
+fi
+
 case "$action" in
 continue)
 	# Sanity check
@@ -338,6 +349,9 @@ abort)
 	rm -r "$state_dir"
 	exit
 	;;
+edit)
+	run_specific_rebase
+  ;;
 esac
 
 # Make sure no rebase is in progress
-- 
1.7.12.289.g0ce9864.dirty

Re: [PATCH] rebase -i: Teach "--edit" action

[Matthieu Moy]

Andrew Wong <andrew.kw.w@gmail.com> writes:

> This allows users to edit the todo list while they're in the middle of
> an interactive rebase.

I like the idea.

> +edit)
> +  git_sequence_editor "$todo" ||
> +    die_abort "Could not execute editor"
> +
> +  exit
> +  ;;

Indent with space. Please, use tabs (same below).

> index 15da926..c394b8d 100755
> --- a/git-rebase.sh
> +++ b/git-rebase.sh
> @@ -38,6 +38,7 @@ C=!                passed to 'git apply'
>  continue!          continue
>  abort!             abort and check out the original branch
>  skip!              skip current patch and continue
> +edit!              edit the todo list during interactive rebase

Just "edit" may be a bit misleading, as we already have the "edit"
action inside the todolist. I'd call this --edit-list to avoid
ambiguity.

This lacks tests, IMHO, as there are many corner-cases (e.g. should we
be allowed to --edit-list while the worktree is in conflict?) that would
deserve to be at least discussed, and as much as possible automatically
tested.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCH] rebase -i: Teach "--edit" action

[Andrew Wong]

On Mon, Sep 10, 2012 at 12:25 PM, Matthieu Moy
<Matthieu.Moy@grenoble-inp.fr> wrote:
> Indent with space. Please, use tabs (same below).

Ah, thanks. Good catch.

> Just "edit" may be a bit misleading, as we already have the "edit"
> action inside the todolist. I'd call this --edit-list to avoid
> ambiguity.

I thought that might be a bit confusing too. "--edit-list" doesn't
seem informative about what "list" we're editing though. What about
"--edit-todo"? Any suggestions are welcomed.

> This lacks tests, IMHO, as there are many corner-cases (e.g. should we
> be allowed to --edit-list while the worktree is in conflict?) that would
> deserve to be at least discussed, and as much as possible automatically
> tested.

It does seem risky to do, since we're exposing something that used to
be internal to "rebase -i". Though I don't see harm in allowing
modifications even when there's a conflict, since we're not really
committing anything, modifying index, or any worktree file. As long as
the todo file exists, and we're stopped in the middle of a rebase, I
think editing it shouldn't cause any problems.

Re: [PATCH] rebase -i: Teach "--edit" action

[Jeff King]

On Mon, Sep 10, 2012 at 12:46:45PM -0400, Andrew Wong wrote:

> > Just "edit" may be a bit misleading, as we already have the "edit"
> > action inside the todolist. I'd call this --edit-list to avoid
> > ambiguity.
> 
> I thought that might be a bit confusing too. "--edit-list" doesn't
> seem informative about what "list" we're editing though. What about
> "--edit-todo"? Any suggestions are welcomed.

Does it ever make sense to edit and then _not_ immediately continue?
You can't affect the current commit anyway (it has already been pulled
from the todo list), so the next thing you'd want to do it actually act
on whatever you put into the todo list[1].

What if it was called --continue-with-edit or something, and then:

> > This lacks tests, IMHO, as there are many corner-cases (e.g. should we
> > be allowed to --edit-list while the worktree is in conflict?) that would
> > deserve to be at least discussed, and as much as possible automatically
> > tested.

We would not even allow the edit if we were not OK to continue.

-Peff

[1] It does preclude using "--edit" to make a note about a later commit
    while you are in the middle of resolving a conflict or something.
    You'd have to do it at the end. I don't know if anybody actually
    cares about that.

Re: [PATCH] rebase -i: Teach "--edit" action

[Johannes Sixt]

Am 10.09.2012 18:54, schrieb Jeff King:
> On Mon, Sep 10, 2012 at 12:46:45PM -0400, Andrew Wong wrote:
> 
>>> Just "edit" may be a bit misleading, as we already have the "edit"
>>> action inside the todolist. I'd call this --edit-list to avoid
>>> ambiguity.
>>
>> I thought that might be a bit confusing too. "--edit-list" doesn't
>> seem informative about what "list" we're editing though. What about
>> "--edit-todo"? Any suggestions are welcomed.
> 
> Does it ever make sense to edit and then _not_ immediately continue?

Yes. For example, while you are resolving a conflict, you might notice
that it would make sense to do something different in the remaining
rebase sequence. You don't want to continue if some conflicts remain.
And you don't want to wait editing the todo list until you are done with
the conflicts because you might have forgotten that you wanted to do
something different.

> You can't affect the current commit anyway (it has already been pulled
> from the todo list), so the next thing you'd want to do it actually act
> on whatever you put into the todo list[1].

Oh, you said it here:

> [1] It does preclude using "--edit" to make a note about a later commit
>     while you are in the middle of resolving a conflict or something.
>     You'd have to do it at the end. I don't know if anybody actually
>     cares about that.

Yes, I do care. At times I tend to have a very short attention span. Or
it is Windows's slowness that expires my short-term memory more often
than not. ;)

-- Hannes

Re: [PATCH] rebase -i: Teach "--edit" action

[Jeff King]

On Mon, Sep 10, 2012 at 08:36:43PM +0200, Johannes Sixt wrote:

> > [1] It does preclude using "--edit" to make a note about a later commit
> >     while you are in the middle of resolving a conflict or something.
> >     You'd have to do it at the end. I don't know if anybody actually
> >     cares about that.
> 
> Yes, I do care. At times I tend to have a very short attention span. Or
> it is Windows's slowness that expires my short-term memory more often
> than not. ;)

OK, then I withdraw my proposal. :)

It sounds like it would be safe to do:

  git rebase --edit-todo
  hack hack hack
  git rebase --continue

anyway, so the restriction is not as valuable as it would otherwise have
been.

-Peff

Re: [PATCH] rebase -i: Teach "--edit" action

[Andrew Wong]

On Mon, Sep 10, 2012 at 2:46 PM, Jeff King <peff@peff.net> wrote:
> On Mon, Sep 10, 2012 at 08:36:43PM +0200, Johannes Sixt wrote:
>
>> > [1] It does preclude using "--edit" to make a note about a later commit
>> >     while you are in the middle of resolving a conflict or something.
>> >     You'd have to do it at the end. I don't know if anybody actually
>> >     cares about that.
>>
>> Yes, I do care. At times I tend to have a very short attention span. Or
>> it is Windows's slowness that expires my short-term memory more often
>> than not. ;)
>
> OK, then I withdraw my proposal. :)
>
> It sounds like it would be safe to do:
>
>   git rebase --edit-todo
>   hack hack hack
>   git rebase --continue

Johannes took the words right out of my mouth.  Also, "edit and _not_
continue" also gives the user a chance to second guess while editing
the todo.

That got me thinking... Currently, the todo list has this line at the bottome:
    # However, if you remove everything, the rebase will be aborted.

We'd probably want to remove that line, since "remove everything" no
longer aborts the rebase. It'll just finish the rebase.  It'll be ugly
to sed it out.  Maybe one way to do this is to remove all the comments
and append new ones.

It might also be nice to add a note to remind the user that they're
editing a todo file in a stopped rebase state. i.e. not a fresh
interactive rebase

Re: [PATCH] rebase -i: Teach "--edit" action

[Junio C Hamano]

Andrew Wong <andrew.kw.w@gmail.com> writes:

> On Mon, Sep 10, 2012 at 2:46 PM, Jeff King <peff@peff.net> wrote:
>> On Mon, Sep 10, 2012 at 08:36:43PM +0200, Johannes Sixt wrote:
>>
>>> > [1] It does preclude using "--edit" to make a note about a later commit
>>> >     while you are in the middle of resolving a conflict or something.
>>> >     You'd have to do it at the end. I don't know if anybody actually
>>> >     cares about that.
>>>
>>> Yes, I do care. At times I tend to have a very short attention span. Or
>>> it is Windows's slowness that expires my short-term memory more often
>>> than not. ;)
>>
>> OK, then I withdraw my proposal. :)
>>
>> It sounds like it would be safe to do:
>>
>>   git rebase --edit-todo
>>   hack hack hack
>>   git rebase --continue
>
> Johannes took the words right out of my mouth.  Also, "edit and _not_
> continue" also gives the user a chance to second guess while editing
> the todo.

do you mean "double check"?

> That got me thinking... Currently, the todo list has this line at the bottome:
>     # However, if you remove everything, the rebase will be aborted.
>
> We'd probably want to remove that line, since "remove everything" no
> longer aborts the rebase. It'll just finish the rebase.

Good precaution.

> It might also be nice to add a note to remind the user that they're
> editing a todo file in a stopped rebase state. i.e. not a fresh
> interactive rebase

Hrm...  They see the contents of the todo file immediately after
they say "rebase --edit-todo" and the sole reason they said that
command is because they wanted to edit the todo file.  Is it likely
they need a reminder?

Re: [PATCH] rebase -i: Teach "--edit" action

[Andrew Wong]

On Mon, Sep 10, 2012 at 3:57 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Hrm...  They see the contents of the todo file immediately after
> they say "rebase --edit-todo" and the sole reason they said that
> command is because they wanted to edit the todo file.  Is it likely
> they need a reminder?

Yes, it's not very likely, but sometimes the todo file takes a bit of
time to finalize.  So there's a good chance that the user can get
interrupted, context switched, or went to do some double checking. And
when the user returns to the editor, it's difficult to tell whether
he's in a fresh rebase or a stopped rebase, unless he remembers.  It's
an unlikely scenario, but if it does happen, I think a short reminder
could avoid some user panic.

I don't plan to change how the todo file looks for a fresh rebase.
I'll probably just add something like this for the stopped rebase
case:
     # You are editing the todo of an ongoing rebase. To continue
rebase after editing, run: "git rebase --continue"

That will also remind the user to run "--continue" afterwards.

[PATCH 0/3] rebase -i: Teach "--edit-todo"

[Andrew Wong]

The flag will allow the user to edit the todo file while they're in the middle
of an interactive rebase. It simply invoke the editor and do nothing else.

More discussions here:
    http://thread.gmane.org/gmane.comp.version-control.git/205133/focus=205182

Andrew Wong (3):
  rebase -i: Refactor help messages for todo file
  rebase -i: Teach "--edit-todo" action
  rebase -i: Add tests for "--edit-todo"

 Documentation/git-rebase.txt  |  5 ++++-
 git-rebase--interactive.sh    | 47 +++++++++++++++++++++++++++++++++----------
 git-rebase.sh                 | 14 +++++++++++++
 t/t3404-rebase-interactive.sh | 16 +++++++++++++++
 4 files changed, 70 insertions(+), 12 deletions(-)

-- 
1.7.12.318.g08bf5ad.dirty

[PATCH 1/3] rebase -i: Refactor help messages for todo file

[Andrew Wong]

Signed-off-by: Andrew Wong <andrew.kw.w@gmail.com>
---
 git-rebase--interactive.sh | 31 ++++++++++++++++++++-----------
 1 file changed, 20 insertions(+), 11 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index a09e842..4d57e50 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -115,6 +115,23 @@ mark_action_done () {
 	fi
 }
 
+append_todo_help () {
+	cat >> "$todo" << EOF
+#
+# Commands:
+#  p, pick = use commit
+#  r, reword = use commit, but edit the commit message
+#  e, edit = use commit, but stop for amending
+#  s, squash = use commit, but meld into previous commit
+#  f, fixup = like "squash", but discard this commit's log message
+#  x, exec = run command (the rest of the line) using shell
+#
+# These lines can be re-ordered; they are executed from top to bottom.
+#
+# If you remove a line here THAT COMMIT WILL BE LOST.
+EOF
+}
+
 make_patch () {
 	sha1_and_parents="$(git rev-list --parents -1 "$1")"
 	case "$sha1_and_parents" in
@@ -901,18 +918,10 @@ test -n "$cmd" && add_exec_commands "$todo"
 cat >> "$todo" << EOF
 
 # Rebase $shortrevisions onto $shortonto
+EOF
+append_todo_help
+cat >> "$todo" << EOF
 #
-# Commands:
-#  p, pick = use commit
-#  r, reword = use commit, but edit the commit message
-#  e, edit = use commit, but stop for amending
-#  s, squash = use commit, but meld into previous commit
-#  f, fixup = like "squash", but discard this commit's log message
-#  x, exec = run command (the rest of the line) using shell
-#
-# These lines can be re-ordered; they are executed from top to bottom.
-#
-# If you remove a line here THAT COMMIT WILL BE LOST.
 # However, if you remove everything, the rebase will be aborted.
 #
 EOF
-- 
1.7.12.318.g08bf5ad.dirty

[PATCH 2/3] rebase -i: Teach "--edit-todo" action

[Andrew Wong]

This allows users to edit the todo file while they're stopped in the
middle of an interactive rebase. When this action is executed, all
comments from the original todo file are stripped, and new help messages
are appended to the end.

Signed-off-by: Andrew Wong <andrew.kw.w@gmail.com>
---
 Documentation/git-rebase.txt |  5 ++++-
 git-rebase--interactive.sh   | 16 ++++++++++++++++
 git-rebase.sh                | 14 ++++++++++++++
 3 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index fd535b0..da067ec 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -12,7 +12,7 @@ SYNOPSIS
 	[<upstream>] [<branch>]
 'git rebase' [-i | --interactive] [options] [--exec <cmd>] [--onto <newbase>]
 	--root [<branch>]
-'git rebase' --continue | --skip | --abort
+'git rebase' --continue | --skip | --abort | --edit-todo
 
 DESCRIPTION
 -----------
@@ -245,6 +245,9 @@ leave out at most one of A and B, in which case it defaults to HEAD.
 --skip::
 	Restart the rebasing process by skipping the current patch.
 
+--edit-todo::
+	Edit the todo list during an interactive rebase.
+
 -m::
 --merge::
 	Use merging strategies to rebase.  When the recursive (default) merge
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 4d57e50..ca55fac 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -792,6 +792,22 @@ skip)
 
 	do_rest
 	;;
+edit-todo)
+	sed -i '/^#/d' "$todo"
+	append_todo_help
+	cat >> "$todo" << EOF
+#
+# You are editing the todo file of an ongoing interactive rebase.
+# To continue rebase after editing, run:
+#     git rebase --continue
+#
+EOF
+
+	git_sequence_editor "$todo" ||
+	die_abort "Could not execute editor"
+
+	exit
+	;;
 esac
 
 git var GIT_COMMITTER_IDENT >/dev/null ||
diff --git a/git-rebase.sh b/git-rebase.sh
index 15da926..e660203 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -38,6 +38,7 @@ C=!                passed to 'git apply'
 continue!          continue
 abort!             abort and check out the original branch
 skip!              skip current patch and continue
+edit-todo!         edit the todo list during an interactive rebase
 "
 . git-sh-setup
 . git-sh-i18n
@@ -194,6 +195,10 @@ do
 		test $total_argc -eq 2 || usage
 		action=${1##--}
 		;;
+	--edit-todo)
+		test $total_argc -eq 2 || usage
+		action=${1##--}
+		;;
 	--onto)
 		test 2 -le "$#" || usage
 		onto="$2"
@@ -306,6 +311,12 @@ then
 	fi
 fi
 
+if test "$action" = "edit-todo" &&
+	test "$type" != "interactive"
+then
+	die "$(gettext "The --edit-todo action can only be used during interactive rebase.")"
+fi
+
 case "$action" in
 continue)
 	# Sanity check
@@ -338,6 +349,9 @@ abort)
 	rm -r "$state_dir"
 	exit
 	;;
+edit-todo)
+	run_specific_rebase
+	;;
 esac
 
 # Make sure no rebase is in progress
-- 
1.7.12.318.g08bf5ad.dirty

Re: [PATCH 2/3] rebase -i: Teach "--edit-todo" action

[Junio C Hamano]

Andrew Wong <andrew.kw.w@gmail.com> writes:

> This allows users to edit the todo file while they're stopped in the
> middle of an interactive rebase. When this action is executed, all
> comments from the original todo file are stripped, and new help messages
> are appended to the end.
>
> Signed-off-by: Andrew Wong <andrew.kw.w@gmail.com>
> ---
>  Documentation/git-rebase.txt |  5 ++++-
>  git-rebase--interactive.sh   | 16 ++++++++++++++++
>  git-rebase.sh                | 14 ++++++++++++++
>  3 files changed, 34 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
> index fd535b0..da067ec 100644
> --- a/Documentation/git-rebase.txt
> +++ b/Documentation/git-rebase.txt
> @@ -12,7 +12,7 @@ SYNOPSIS
>  	[<upstream>] [<branch>]
>  'git rebase' [-i | --interactive] [options] [--exec <cmd>] [--onto <newbase>]
>  	--root [<branch>]
> -'git rebase' --continue | --skip | --abort
> +'git rebase' --continue | --skip | --abort | --edit-todo
>  
>  DESCRIPTION
>  -----------
> @@ -245,6 +245,9 @@ leave out at most one of A and B, in which case it defaults to HEAD.
>  --skip::
>  	Restart the rebasing process by skipping the current patch.
>  
> +--edit-todo::
> +	Edit the todo list during an interactive rebase.
> +
>  -m::
>  --merge::
>  	Use merging strategies to rebase.  When the recursive (default) merge
> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index 4d57e50..ca55fac 100644
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -792,6 +792,22 @@ skip)
>  
>  	do_rest
>  	;;
> +edit-todo)
> +	sed -i '/^#/d' "$todo"
> +	append_todo_help

I do not think the "sed -i" in-place edit is portable.  AFAIK, it is
not even in POSIX.1

In any case, what information are you discarding and then replacing
with the standard boilerplate?

> diff --git a/git-rebase.sh b/git-rebase.sh
> index 15da926..e660203 100755
> --- a/git-rebase.sh
> +++ b/git-rebase.sh
> @@ -38,6 +38,7 @@ C=!                passed to 'git apply'
>  continue!          continue
>  abort!             abort and check out the original branch
>  skip!              skip current patch and continue
> +edit-todo!         edit the todo list during an interactive rebase
>  "
>  . git-sh-setup
>  . git-sh-i18n
> @@ -194,6 +195,10 @@ do
>  		test $total_argc -eq 2 || usage
>  		action=${1##--}
>  		;;
> +	--edit-todo)
> +		test $total_argc -eq 2 || usage
> +		action=${1##--}
> +		;;
>  	--onto)
>  		test 2 -le "$#" || usage
>  		onto="$2"
> @@ -306,6 +311,12 @@ then
>  	fi
>  fi
>  
> +if test "$action" = "edit-todo" &&
> +	test "$type" != "interactive"

I do not see a need to split this short expression into two lines.

Re: [PATCH 2/3] rebase -i: Teach "--edit-todo" action

[Andrew Wong]

On 09/16/12 02:54, Junio C Hamano wrote:
> In any case, what information are you discarding and then replacing
> with the standard boilerplate?
It's to strip out the comment that says:

    # However, if you remove everything, the rebase will be aborted.

As there's no way reliable way to know where that line is and remove it,
the only way I can think of is to remove all the comments, and append
the help messages again.

Re: [PATCH 2/3] rebase -i: Teach "--edit-todo" action

[Junio C Hamano]

Andrew Wong <andrew.kw.w@gmail.com> writes:

> On 09/16/12 02:54, Junio C Hamano wrote:
>> In any case, what information are you discarding and then replacing
>> with the standard boilerplate?
> It's to strip out the comment that says:
>
>     # However, if you remove everything, the rebase will be aborted.
>
> As there's no way reliable way to know where that line is and remove it,
> the only way I can think of is to remove all the comments, and append
> the help messages again.

I see.  As long as you know that the other things you are removing
with the code is irrelevant and giving the standard boilerplate is
sufficient, your approach is the simplest way to reliably get what
you want.

Thanks.

[PATCH 3/3] rebase -i: Add tests for "--edit-todo"

[Andrew Wong]

Signed-off-by: Andrew Wong <andrew.kw.w@gmail.com>
---
 t/t3404-rebase-interactive.sh | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 7304b66..a194c97 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -911,4 +911,20 @@ test_expect_success 'rebase -i --root fixup root commit' '
 	test 0 = $(git cat-file commit HEAD | grep -c ^parent\ )
 '
 
+test_expect_success 'rebase --edit-todo does not works on non-interactive rebase' '
+	git checkout conflict-branch &&
+	test_must_fail git rebase --onto HEAD~2 HEAD~ &&
+	test_must_fail git rebase --edit-todo &&
+	git rebase --abort
+'
+
+test_expect_success 'rebase --edit-todo can be used to modify todo' '
+	git checkout no-conflict-branch^0 &&
+	FAKE_LINES="edit 1 2 3" git rebase -i HEAD~3 &&
+	FAKE_LINES="2 1" git rebase --edit-todo &&
+	git rebase --continue
+	test M = $(git cat-file commit HEAD^ | sed -ne \$p) &&
+	test L = $(git cat-file commit HEAD | sed -ne \$p)
+'
+
 test_done
-- 
1.7.12.318.g08bf5ad.dirty

Re: [PATCH 3/3] rebase -i: Add tests for "--edit-todo"

[Junio C Hamano]

Andrew Wong <andrew.kw.w@gmail.com> writes:

> Signed-off-by: Andrew Wong <andrew.kw.w@gmail.com>
> ---
>  t/t3404-rebase-interactive.sh | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
>
> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
> index 7304b66..a194c97 100755
> --- a/t/t3404-rebase-interactive.sh
> +++ b/t/t3404-rebase-interactive.sh
> @@ -911,4 +911,20 @@ test_expect_success 'rebase -i --root fixup root commit' '
>  	test 0 = $(git cat-file commit HEAD | grep -c ^parent\ )
>  '
>  
> +test_expect_success 'rebase --edit-todo does not works on non-interactive rebase' '
> +	git checkout conflict-branch &&
> +	test_must_fail git rebase --onto HEAD~2 HEAD~ &&
> +	test_must_fail git rebase --edit-todo &&
> +	git rebase --abort
> +'

It _might_ be that you simply inherited sloppiness from surrounding
existing tests, but what happens when a test _before_ this test
failed?  Is "git checkout conflict-branch" sufficient to bring you
to a sensible state where this test would succeed?  I'd prefer to
see a defensive "git reset --hard &&" before the first "checkout".

The same for the next one.

> +test_expect_success 'rebase --edit-todo can be used to modify todo' '
> +	git checkout no-conflict-branch^0 &&
> +	FAKE_LINES="edit 1 2 3" git rebase -i HEAD~3 &&
> +	FAKE_LINES="2 1" git rebase --edit-todo &&
> +	git rebase --continue
> +	test M = $(git cat-file commit HEAD^ | sed -ne \$p) &&
> +	test L = $(git cat-file commit HEAD | sed -ne \$p)
> +'
> +
>  test_done

[PATCH v2 1/3] rebase -i: Refactor help messages for todo file

[Andrew Wong]

Signed-off-by: Andrew Wong <andrew.kw.w@gmail.com>
---
 git-rebase--interactive.sh | 31 ++++++++++++++++++++-----------
 1 file changed, 20 insertions(+), 11 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index a09e842..4d57e50 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -115,6 +115,23 @@ mark_action_done () {
 	fi
 }
 
+append_todo_help () {
+	cat >> "$todo" << EOF
+#
+# Commands:
+#  p, pick = use commit
+#  r, reword = use commit, but edit the commit message
+#  e, edit = use commit, but stop for amending
+#  s, squash = use commit, but meld into previous commit
+#  f, fixup = like "squash", but discard this commit's log message
+#  x, exec = run command (the rest of the line) using shell
+#
+# These lines can be re-ordered; they are executed from top to bottom.
+#
+# If you remove a line here THAT COMMIT WILL BE LOST.
+EOF
+}
+
 make_patch () {
 	sha1_and_parents="$(git rev-list --parents -1 "$1")"
 	case "$sha1_and_parents" in
@@ -901,18 +918,10 @@ test -n "$cmd" && add_exec_commands "$todo"
 cat >> "$todo" << EOF
 
 # Rebase $shortrevisions onto $shortonto
+EOF
+append_todo_help
+cat >> "$todo" << EOF
 #
-# Commands:
-#  p, pick = use commit
-#  r, reword = use commit, but edit the commit message
-#  e, edit = use commit, but stop for amending
-#  s, squash = use commit, but meld into previous commit
-#  f, fixup = like "squash", but discard this commit's log message
-#  x, exec = run command (the rest of the line) using shell
-#
-# These lines can be re-ordered; they are executed from top to bottom.
-#
-# If you remove a line here THAT COMMIT WILL BE LOST.
 # However, if you remove everything, the rebase will be aborted.
 #
 EOF
-- 
1.7.12.318.g79683ba.dirty

[PATCH v2 2/3] rebase -i: Teach "--edit-todo" action

[Andrew Wong]

This allows users to edit the todo file while they're stopped in the
middle of an interactive rebase. When this action is executed, all
comments from the original todo file are stripped, and new help messages
are appended to the end.

Signed-off-by: Andrew Wong <andrew.kw.w@gmail.com>
---
 Documentation/git-rebase.txt |  5 ++++-
 git-rebase--interactive.sh   | 17 +++++++++++++++++
 git-rebase.sh                | 13 +++++++++++++
 3 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index fd535b0..da067ec 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -12,7 +12,7 @@ SYNOPSIS
 	[<upstream>] [<branch>]
 'git rebase' [-i | --interactive] [options] [--exec <cmd>] [--onto <newbase>]
 	--root [<branch>]
-'git rebase' --continue | --skip | --abort
+'git rebase' --continue | --skip | --abort | --edit-todo
 
 DESCRIPTION
 -----------
@@ -245,6 +245,9 @@ leave out at most one of A and B, in which case it defaults to HEAD.
 --skip::
 	Restart the rebasing process by skipping the current patch.
 
+--edit-todo::
+	Edit the todo list during an interactive rebase.
+
 -m::
 --merge::
 	Use merging strategies to rebase.  When the recursive (default) merge
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 4d57e50..fcd15be 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -792,6 +792,23 @@ skip)
 
 	do_rest
 	;;
+edit-todo)
+	sed -e '/^#/d' < "$todo" > "$todo".new
+	mv -f "$todo".new "$todo"
+	append_todo_help
+	cat >> "$todo" << EOF
+#
+# You are editing the todo file of an ongoing interactive rebase.
+# To continue rebase after editing, run:
+#     git rebase --continue
+#
+EOF
+
+	git_sequence_editor "$todo" ||
+	die_abort "Could not execute editor"
+
+	exit
+	;;
 esac
 
 git var GIT_COMMITTER_IDENT >/dev/null ||
diff --git a/git-rebase.sh b/git-rebase.sh
index 15da926..e5a289c 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -38,6 +38,7 @@ C=!                passed to 'git apply'
 continue!          continue
 abort!             abort and check out the original branch
 skip!              skip current patch and continue
+edit-todo!         edit the todo list during an interactive rebase
 "
 . git-sh-setup
 . git-sh-i18n
@@ -194,6 +195,10 @@ do
 		test $total_argc -eq 2 || usage
 		action=${1##--}
 		;;
+	--edit-todo)
+		test $total_argc -eq 2 || usage
+		action=${1##--}
+		;;
 	--onto)
 		test 2 -le "$#" || usage
 		onto="$2"
@@ -306,6 +311,11 @@ then
 	fi
 fi
 
+if test "$action" = "edit-todo" && test "$type" != "interactive"
+then
+	die "$(gettext "The --edit-todo action can only be used during interactive rebase.")"
+fi
+
 case "$action" in
 continue)
 	# Sanity check
@@ -338,6 +348,9 @@ abort)
 	rm -r "$state_dir"
 	exit
 	;;
+edit-todo)
+	run_specific_rebase
+	;;
 esac
 
 # Make sure no rebase is in progress
-- 
1.7.12.318.g79683ba.dirty

Re: [PATCH v2 2/3] rebase -i: Teach "--edit-todo" action

[Martin von Zweigbergk]

On Sun, Sep 16, 2012 at 8:17 AM, Andrew Wong <andrew.kw.w@gmail.com> wrote:
> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
> index fd535b0..da067ec 100644
> --- a/Documentation/git-rebase.txt
> +++ b/Documentation/git-rebase.txt
> @@ -12,7 +12,7 @@ SYNOPSIS
>         [<upstream>] [<branch>]
>  'git rebase' [-i | --interactive] [options] [--exec <cmd>] [--onto <newbase>]
>         --root [<branch>]
> -'git rebase' --continue | --skip | --abort
> +'git rebase' --continue | --skip | --abort | --edit-todo

I guess you should add --edit-todo to OPTIONS_SPEC in git-rebase.sh as
well. The OPTIONS_SPEC needs another little update too. I have
included a patch at the end of this email that you include in a
re-roll.

> +       git_sequence_editor "$todo" ||
> +       die_abort "Could not execute editor"

die_abort seems a little harsh -- it will discard the rebase state.
Plain "die" would be better, I think.

Also, if you even need to break the line after the || operator, you
might want to indent the remainder by one tab. This file is quite
consistent in using that style, although I don't know what the
preferred style is in general in git.

>  git var GIT_COMMITTER_IDENT >/dev/null ||
> diff --git a/git-rebase.sh b/git-rebase.sh
> index 15da926..e5a289c 100755
> --- a/git-rebase.sh
> +++ b/git-rebase.sh
> @@ -194,6 +195,10 @@ do
>                 test $total_argc -eq 2 || usage
>                 action=${1##--}
>                 ;;
> +       --edit-todo)
> +               test $total_argc -eq 2 || usage
> +               action=${1##--}
> +               ;;

It looks like this could be trivially combined with the previous case
arm, making the match "--continue|--skip|--abort|--edit-todo)".


-->8--
Author: Martin von Zweigbergk <martinvonz@gmail.com>

    rebase usage: subcommands can not be combined with -i

    Since 95135b0 (rebase: stricter check of standalone sub command,
    2011-02-06), git-rebase has not allowed to use -i together with
    e.g. --continue. Yet, when rebase started using OPTIONS_SPEC in
    45e2acf (rebase: define options in OPTIONS_SPEC, 2011-02-28), the
    usage message included

      git-rebase [-i] --continue | --abort | --skip

    Remove the "[-i]" from this line.

    Signed-off-by: Martin von Zweigbergk <martinvonz@gmail.com>

diff --git a/git-rebase.sh b/git-rebase.sh
index 15da926..e6b43a2 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -8,7 +8,7 @@ OPTIONS_KEEPDASHDASH=
 OPTIONS_SPEC="\
 git rebase [-i] [options] [--exec <cmd>] [--onto <newbase>]
[<upstream>] [<branch>]
 git rebase [-i] [options] [--exec <cmd>] [--onto <newbase>] --root [<branch>]
-git-rebase [-i] --continue | --abort | --skip
+git-rebase --continue | --abort | --skip
 --
  Available options are
 v,verbose!         display a diffstat of what changed upstream

[PATCH v3 0/4] rebase -i: Teach "--edit-todo" action

[Andrew Wong]

Made the fixes as suggested by Martin.

Martin: Good points. Thanks!

Andrew Wong (3):
  rebase -i: Refactor help messages for todo file
  rebase -i: Teach "--edit-todo" action
  rebase -i: Add tests for "--edit-todo"

Martin von Zweigbergk (1):
  rebase usage: subcommands can not be combined with -i

 Documentation/git-rebase.txt  |  5 ++++-
 git-rebase--interactive.sh    | 48 +++++++++++++++++++++++++++++++++----------
 git-rebase.sh                 | 13 ++++++++++--
 t/t3404-rebase-interactive.sh | 18 ++++++++++++++++
 4 files changed, 70 insertions(+), 14 deletions(-)

-- 
1.7.12.318.g79683ba.dirty

[PATCH 1/4] rebase usage: subcommands can not be combined with -i

[Andrew Wong]

From: Martin von Zweigbergk <martinvonz@gmail.com>

Since 95135b0 (rebase: stricter check of standalone sub command,
2011-02-06), git-rebase has not allowed to use -i together with e.g.
--continue. Yet, when rebase started using OPTIONS_SPEC in 45e2acf
(rebase: define options in OPTIONS_SPEC, 2011-02-28), the usage message
included

	git-rebase [-i] --continue | --abort | --skip

Remove the "[-i]" from this line.

Signed-off-by: Martin von Zweigbergk <martinvonz@gmail.com>
Signed-off-by: Andrew Wong <andrew.kw.w@gmail.com>
---
 git-rebase.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/git-rebase.sh b/git-rebase.sh
index 15da926..e6b43a2 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -8,7 +8,7 @@ OPTIONS_KEEPDASHDASH=
 OPTIONS_SPEC="\
 git rebase [-i] [options] [--exec <cmd>] [--onto <newbase>] [<upstream>] [<branch>]
 git rebase [-i] [options] [--exec <cmd>] [--onto <newbase>] --root [<branch>]
-git-rebase [-i] --continue | --abort | --skip
+git-rebase --continue | --abort | --skip
 --
  Available options are
 v,verbose!         display a diffstat of what changed upstream
-- 
1.7.12.318.g79683ba.dirty

[PATCH 2/4] rebase -i: Refactor help messages for todo file

[Andrew Wong]

Signed-off-by: Andrew Wong <andrew.kw.w@gmail.com>
---
 git-rebase--interactive.sh | 31 ++++++++++++++++++++-----------
 1 file changed, 20 insertions(+), 11 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index a09e842..4d57e50 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -115,6 +115,23 @@ mark_action_done () {
 	fi
 }
 
+append_todo_help () {
+	cat >> "$todo" << EOF
+#
+# Commands:
+#  p, pick = use commit
+#  r, reword = use commit, but edit the commit message
+#  e, edit = use commit, but stop for amending
+#  s, squash = use commit, but meld into previous commit
+#  f, fixup = like "squash", but discard this commit's log message
+#  x, exec = run command (the rest of the line) using shell
+#
+# These lines can be re-ordered; they are executed from top to bottom.
+#
+# If you remove a line here THAT COMMIT WILL BE LOST.
+EOF
+}
+
 make_patch () {
 	sha1_and_parents="$(git rev-list --parents -1 "$1")"
 	case "$sha1_and_parents" in
@@ -901,18 +918,10 @@ test -n "$cmd" && add_exec_commands "$todo"
 cat >> "$todo" << EOF
 
 # Rebase $shortrevisions onto $shortonto
+EOF
+append_todo_help
+cat >> "$todo" << EOF
 #
-# Commands:
-#  p, pick = use commit
-#  r, reword = use commit, but edit the commit message
-#  e, edit = use commit, but stop for amending
-#  s, squash = use commit, but meld into previous commit
-#  f, fixup = like "squash", but discard this commit's log message
-#  x, exec = run command (the rest of the line) using shell
-#
-# These lines can be re-ordered; they are executed from top to bottom.
-#
-# If you remove a line here THAT COMMIT WILL BE LOST.
 # However, if you remove everything, the rebase will be aborted.
 #
 EOF
-- 
1.7.12.318.g79683ba.dirty

[PATCH 3/4] rebase -i: Teach "--edit-todo" action

[Andrew Wong]

This allows users to edit the todo file while they're stopped in the
middle of an interactive rebase. When this action is executed, all
comments from the original todo file are stripped, and new help messages
are appended to the end.

Signed-off-by: Andrew Wong <andrew.kw.w@gmail.com>
---
 Documentation/git-rebase.txt |  5 ++++-
 git-rebase--interactive.sh   | 17 +++++++++++++++++
 git-rebase.sh                | 13 +++++++++++--
 3 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index fd535b0..da067ec 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -12,7 +12,7 @@ SYNOPSIS
 	[<upstream>] [<branch>]
 'git rebase' [-i | --interactive] [options] [--exec <cmd>] [--onto <newbase>]
 	--root [<branch>]
-'git rebase' --continue | --skip | --abort
+'git rebase' --continue | --skip | --abort | --edit-todo
 
 DESCRIPTION
 -----------
@@ -245,6 +245,9 @@ leave out at most one of A and B, in which case it defaults to HEAD.
 --skip::
 	Restart the rebasing process by skipping the current patch.
 
+--edit-todo::
+	Edit the todo list during an interactive rebase.
+
 -m::
 --merge::
 	Use merging strategies to rebase.  When the recursive (default) merge
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 4d57e50..2b8f2a9 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -792,6 +792,23 @@ skip)
 
 	do_rest
 	;;
+edit-todo)
+	sed -e '/^#/d' < "$todo" > "$todo".new
+	mv -f "$todo".new "$todo"
+	append_todo_help
+	cat >> "$todo" << EOF
+#
+# You are editing the todo file of an ongoing interactive rebase.
+# To continue rebase after editing, run:
+#     git rebase --continue
+#
+EOF
+
+	git_sequence_editor "$todo" ||
+		die "Could not execute editor"
+
+	exit
+	;;
 esac
 
 git var GIT_COMMITTER_IDENT >/dev/null ||
diff --git a/git-rebase.sh b/git-rebase.sh
index e6b43a2..b2f1c76 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -8,7 +8,7 @@ OPTIONS_KEEPDASHDASH=
 OPTIONS_SPEC="\
 git rebase [-i] [options] [--exec <cmd>] [--onto <newbase>] [<upstream>] [<branch>]
 git rebase [-i] [options] [--exec <cmd>] [--onto <newbase>] --root [<branch>]
-git-rebase --continue | --abort | --skip
+git-rebase --continue | --abort | --skip | --edit-todo
 --
  Available options are
 v,verbose!         display a diffstat of what changed upstream
@@ -38,6 +38,7 @@ C=!                passed to 'git apply'
 continue!          continue
 abort!             abort and check out the original branch
 skip!              skip current patch and continue
+edit-todo!         edit the todo list during an interactive rebase
 "
 . git-sh-setup
 . git-sh-i18n
@@ -190,7 +191,7 @@ do
 	--verify)
 		ok_to_skip_pre_rebase=
 		;;
-	--continue|--skip|--abort)
+	--continue|--skip|--abort|--edit-todo)
 		test $total_argc -eq 2 || usage
 		action=${1##--}
 		;;
@@ -306,6 +307,11 @@ then
 	fi
 fi
 
+if test "$action" = "edit-todo" && test "$type" != "interactive"
+then
+	die "$(gettext "The --edit-todo action can only be used during interactive rebase.")"
+fi
+
 case "$action" in
 continue)
 	# Sanity check
@@ -338,6 +344,9 @@ abort)
 	rm -r "$state_dir"
 	exit
 	;;
+edit-todo)
+	run_specific_rebase
+	;;
 esac
 
 # Make sure no rebase is in progress
-- 
1.7.12.318.g79683ba.dirty

[PATCH 4/4] rebase -i: Add tests for "--edit-todo"

[Andrew Wong]

Signed-off-by: Andrew Wong <andrew.kw.w@gmail.com>
---
 t/t3404-rebase-interactive.sh | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 7304b66..6eafb63 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -911,4 +911,22 @@ test_expect_success 'rebase -i --root fixup root commit' '
 	test 0 = $(git cat-file commit HEAD | grep -c ^parent\ )
 '
 
+test_expect_success 'rebase --edit-todo does not works on non-interactive rebase' '
+	git reset --hard &&
+	git checkout conflict-branch &&
+	test_must_fail git rebase --onto HEAD~2 HEAD~ &&
+	test_must_fail git rebase --edit-todo &&
+	git rebase --abort
+'
+
+test_expect_success 'rebase --edit-todo can be used to modify todo' '
+	git reset --hard &&
+	git checkout no-conflict-branch^0 &&
+	FAKE_LINES="edit 1 2 3" git rebase -i HEAD~3 &&
+	FAKE_LINES="2 1" git rebase --edit-todo &&
+	git rebase --continue
+	test M = $(git cat-file commit HEAD^ | sed -ne \$p) &&
+	test L = $(git cat-file commit HEAD | sed -ne \$p)
+'
+
 test_done
-- 
1.7.12.318.g79683ba.dirty

Re: [PATCH 4/4] rebase -i: Add tests for "--edit-todo"

[Martin von Zweigbergk]

On Mon, Sep 17, 2012 at 6:28 PM, Andrew Wong <andrew.kw.w@gmail.com> wrote:
> +       test M = $(git cat-file commit HEAD^ | sed -ne \$p) &&
> +       test L = $(git cat-file commit HEAD | sed -ne \$p)

I couldn't find "$" (match last line) in the POSIX man page for sed.
Besides, I think $(git show -s --format=%s HEAD) reads better.

For future reference, and if you haven't already, you may want to
install manpages-posix (or manpages-posix-dev?) and then you can run
e.g. "man 1p sed" to see the POSIX man page for sed.

Re: [PATCH 4/4] rebase -i: Add tests for "--edit-todo"

[Andrew Wong]

On 09/18/12 00:58, Martin von Zweigbergk wrote:
> On Mon, Sep 17, 2012 at 6:28 PM, Andrew Wong <andrew.kw.w@gmail.com> wrote:
>> +       test M = $(git cat-file commit HEAD^ | sed -ne \$p) &&
>> +       test L = $(git cat-file commit HEAD | sed -ne \$p)
> I couldn't find "$" (match last line) in the POSIX man page for sed.
> Besides, I think $(git show -s --format=%s HEAD) reads better.
It's under "Addresses in sed":
        ... a '$' character that addresses the last line of input ...
from:
        http://pubs.opengroup.org/onlinepubs/009695399/utilities/sed.html

Various places in that test get the content of files that way, so I
thought it's better to just follow the rest of the test. I agree that
there are better ways to achieve the same thing though.

Re: [PATCH 4/4] rebase -i: Add tests for "--edit-todo"

[Martin von Zweigbergk]

On Mon, Sep 17, 2012 at 10:23 PM, Andrew Wong <andrew.kw.w@gmail.com> wrote:
> On 09/18/12 00:58, Martin von Zweigbergk wrote:
>> On Mon, Sep 17, 2012 at 6:28 PM, Andrew Wong <andrew.kw.w@gmail.com> wrote:
>>> +       test M = $(git cat-file commit HEAD^ | sed -ne \$p) &&
>>> +       test L = $(git cat-file commit HEAD | sed -ne \$p)
>> I couldn't find "$" (match last line) in the POSIX man page for sed.
>> Besides, I think $(git show -s --format=%s HEAD) reads better.
> It's under "Addresses in sed":
>         ... a '$' character that addresses the last line of input ...

Ah, I just didn't look hard enough; sorry. Good to know.

FWIW, Acked-by: Martin von Zweigbergk <martinvonz@gmail.com>

[PATCH 5/4] rebase -i: suggest using --edit-todo to fix an unknown instruction

[Johannes Sixt]

From: Johannes Sixt <j6t@kdbg.org>

We have now an explicit UI to edit the todo sheet and need not disclose
the name of the file.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
 git-rebase--interactive.sh | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 2b8f2a9..4b2ef11 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -575,11 +575,12 @@ do_next () {
 		;;
 	*)
 		warn "Unknown command: $command $sha1 $rest"
+		fixtodo="Please fix this using 'git rebase --edit-todo'."
 		if git rev-parse --verify -q "$sha1" >/dev/null
 		then
-			die_with_patch $sha1 "Please fix this in the file $todo."
+			die_with_patch $sha1 "$fixtodo"
 		else
-			die "Please fix this in the file $todo."
+			die "$fixtodo"
 		fi
 		;;
 	esac
-- 
1.7.12.1721.gd1d8b74

Re: [PATCH 5/4] rebase -i: suggest using --edit-todo to fix an unknown instruction

[Junio C Hamano]

Johannes Sixt <j.sixt@viscovery.net> writes:

> From: Johannes Sixt <j6t@kdbg.org>
>
> We have now an explicit UI to edit the todo sheet and need not disclose
> the name of the file.
>
> Signed-off-by: Johannes Sixt <j6t@kdbg.org>
> ---
>  git-rebase--interactive.sh | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index 2b8f2a9..4b2ef11 100644
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -575,11 +575,12 @@ do_next () {
>  		;;
>  	*)
>  		warn "Unknown command: $command $sha1 $rest"
> +		fixtodo="Please fix this using 'git rebase --edit-todo'."
>  		if git rev-parse --verify -q "$sha1" >/dev/null
>  		then
> -			die_with_patch $sha1 "Please fix this in the file $todo."
> +			die_with_patch $sha1 "$fixtodo"
>  		else
> -			die "Please fix this in the file $todo."
> +			die "$fixtodo"
>  		fi
>  		;;
>  	esac

Good thinking. Makes sense to me.

[PATCH v2 3/3] rebase -i: Add tests for "--edit-todo"

[Andrew Wong]

Signed-off-by: Andrew Wong <andrew.kw.w@gmail.com>
---
 t/t3404-rebase-interactive.sh | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 7304b66..6eafb63 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -911,4 +911,22 @@ test_expect_success 'rebase -i --root fixup root commit' '
 	test 0 = $(git cat-file commit HEAD | grep -c ^parent\ )
 '
 
+test_expect_success 'rebase --edit-todo does not works on non-interactive rebase' '
+	git reset --hard &&
+	git checkout conflict-branch &&
+	test_must_fail git rebase --onto HEAD~2 HEAD~ &&
+	test_must_fail git rebase --edit-todo &&
+	git rebase --abort
+'
+
+test_expect_success 'rebase --edit-todo can be used to modify todo' '
+	git reset --hard &&
+	git checkout no-conflict-branch^0 &&
+	FAKE_LINES="edit 1 2 3" git rebase -i HEAD~3 &&
+	FAKE_LINES="2 1" git rebase --edit-todo &&
+	git rebase --continue
+	test M = $(git cat-file commit HEAD^ | sed -ne \$p) &&
+	test L = $(git cat-file commit HEAD | sed -ne \$p)
+'
+
 test_done
-- 
1.7.12.318.g79683ba.dirty

Re: [RFC] Add "edit" action for interactive rebase?

[Johannes Sixt]

Am 10.09.2012 18:14, schrieb Andrew Wong:
> Occasionally, while I'm in the middle of an interactive rebase, I'd change my
> mind about the todo list and want to modify it.  This means manually digging
> out the todo file from the rebase directory, and invoking the editor.  So I
> thought it might be convenient to have an "edit" action that simply invokes the
> editor on the todo file but do nothing else.
> 
> This should be safe to do in the middle of a rebase, since we don't preprocess
> the todo file and generate any state from it.  I've also been manually editing
> the todo file a while now, and I never ran into any issues.
> 
> I wonder if any others have ever ran into this situation, and would this be
> a useful feature to have in interactive rebase? Comments?

Applause! A very welcome addition. I've found myself editing the todo
list every now and then, and I'd like to do that more often. This new
feature would make it dead simple.

Did you think about what can go wrong? For example, starting with this
todo sheet:

  exec false
  pick 1234567

and then the user changes the 'pick' to 'squash' after rebase stopped at
the failed 'exec' command.

-- Hannes

Re: [RFC] Add "edit" action for interactive rebase?

[Andrew Wong]

On Mon, Sep 10, 2012 at 2:28 PM, Johannes Sixt <j6t@kdbg.org> wrote:
> Did you think about what can go wrong? For example, starting with this
> todo sheet:
>
>   exec false
>   pick 1234567

Ah, that's definitely a problem.

I was going to say we probably just to check the "done" file, same as
the one we do for a fresh "rebase -i", but it turns out the "exec
false" will fool the "has_action" check for a fresh "rebase -i" too.
Heh.

Maybe we should improve the check for a fresh "rebase -i" case, then
we can do the same check for this case. Maybe we can grep for a "pick"
in "done" file? Or we can check if there's anything in "rewritten"?
Though I'm not sure if any of those is really foolproof. Or should we
just ignore this case and assume the user knows what s/he's doing?

Incidentally, if the starting todo file is:
    pick A
    exec false
    pick B

If the user then changes the "pick B" to "squash B", it should be a
valid I think, and "rebase -i" should handle that properly. It should,
because that's the same thing as:
    pick C (which results in a conflict and stopped)
    squash D

OT: That "exec false" !
I ran into numerous occasions where I wanted to manually do something
before the "first commit after upstream", such as creating a new
commit or merge.  And I only had two ways of doing it:
1. to rebase against "upstream^", and then mark the upstream as edit
2. insert a "exec bash" in front of the "first commit"
But "exec false" will work much much nicer. :)