[v2,2/2] rebase -i: introduce the 'break' command
diff mbox series

Message ID c74e02c4b643652d43108c1a5a675df0fae5f059.1539161632.git.gitgitgadget@gmail.com
State New
Headers show
Series
  • rebase -i: introduce the 'break' command
Related show

Commit Message

Utsav Shah via GitGitGadget Oct. 10, 2018, 8:53 a.m. UTC
From: Johannes Schindelin <johannes.schindelin@gmx.de>

The 'edit' command can be used to cherry-pick a commit and then
immediately drop out of the interactive rebase, with exit code 0, to let
the user amend the commit, or test it, or look around.

Sometimes this functionality would come in handy *without*
cherry-picking a commit, e.g. to interrupt the interactive rebase even
before cherry-picking a commit, or immediately after an 'exec' or a
'merge'.

This commit introduces that functionality, as the spanking new 'break'
command.

Suggested-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 Documentation/git-rebase.txt | 3 +++
 rebase-interactive.c         | 1 +
 sequencer.c                  | 7 ++++++-
 t/lib-rebase.sh              | 2 +-
 t/t3418-rebase-continue.sh   | 9 +++++++++
 5 files changed, 20 insertions(+), 2 deletions(-)

Comments

Phillip Wood Oct. 11, 2018, 9:08 a.m. UTC | #1
Hi Johannes

I think this would be a useful addition to rebase, there's one small
comment below.

On 10/10/2018 09:53, Johannes Schindelin via GitGitGadget wrote:
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
> 
> The 'edit' command can be used to cherry-pick a commit and then
> immediately drop out of the interactive rebase, with exit code 0, to let
> the user amend the commit, or test it, or look around.
> 
> Sometimes this functionality would come in handy *without*
> cherry-picking a commit, e.g. to interrupt the interactive rebase even
> before cherry-picking a commit, or immediately after an 'exec' or a
> 'merge'.
> 
> This commit introduces that functionality, as the spanking new 'break'
> command.
> 
> Suggested-by: Stefan Beller <sbeller@google.com>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  Documentation/git-rebase.txt | 3 +++
>  rebase-interactive.c         | 1 +
>  sequencer.c                  | 7 ++++++-
>  t/lib-rebase.sh              | 2 +-
>  t/t3418-rebase-continue.sh   | 9 +++++++++
>  5 files changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
> index db2faca73c..5bed1da36b 100644
> --- a/Documentation/git-rebase.txt
> +++ b/Documentation/git-rebase.txt
> @@ -561,6 +561,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.
>  
> +To interrupt the rebase (just like an "edit" command would do, but without
> +cherry-picking any commit first), use the "break" command.
> +
>  If you just want to edit the commit message for a commit, replace the
>  command "pick" with the command "reword".
>  
> diff --git a/rebase-interactive.c b/rebase-interactive.c
> index 0f4119cbae..78f3263fc1 100644
> --- a/rebase-interactive.c
> +++ b/rebase-interactive.c
> @@ -14,6 +14,7 @@ void append_todo_help(unsigned edit_todo, unsigned keep_empty,
>  "s, squash <commit> = use commit, but meld into previous commit\n"
>  "f, fixup <commit> = like \"squash\", but discard this commit's log message\n"
>  "x, exec <command> = run command (the rest of the line) using shell\n"
> +"b, break = stop here (continue rebase later with 'git rebase --continue')\n"
>  "d, drop <commit> = remove commit\n"
>  "l, label <label> = label current HEAD with a name\n"
>  "t, reset <label> = reset HEAD to a label\n"
> diff --git a/sequencer.c b/sequencer.c
> index 8dd6db5a01..b209f8af46 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -1416,6 +1416,7 @@ enum todo_command {
>  	TODO_SQUASH,
>  	/* commands that do something else than handling a single commit */
>  	TODO_EXEC,
> +	TODO_BREAK,
>  	TODO_LABEL,
>  	TODO_RESET,
>  	TODO_MERGE,
> @@ -1437,6 +1438,7 @@ static struct {
>  	{ 'f', "fixup" },
>  	{ 's', "squash" },
>  	{ 'x', "exec" },
> +	{ 'b', "break" },
>  	{ 'l', "label" },
>  	{ 't', "reset" },
>  	{ 'm', "merge" },
> @@ -1964,7 +1966,7 @@ static int parse_insn_line(struct todo_item *item, const char *bol, char *eol)
>  	padding = strspn(bol, " \t");
>  	bol += padding;
>  
> -	if (item->command == TODO_NOOP) {
> +	if (item->command == TODO_NOOP || item->command == TODO_BREAK) {
>  		if (bol != eol)
>  			return error(_("%s does not accept arguments: '%s'"),
>  				     command_to_string(item->command), bol);
> @@ -3293,6 +3295,9 @@ static int pick_commits(struct todo_list *todo_list, struct replay_opts *opts)
>  			unlink(rebase_path_stopped_sha());
>  			unlink(rebase_path_amend());
>  			delete_ref(NULL, "REBASE_HEAD", NULL, REF_NO_DEREF);
> +
> +			if (item->command == TODO_BREAK)
> +				break;

Normally when rebase stops it prints a message to say where it has got
to and how to continue, it might be useful to do the same here

Best Wishes

Phillip

>  		}
>  		if (item->command <= TODO_SQUASH) {
>  			if (is_rebase_i(opts))
> diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh
> index 25a77ee5cb..584604ee63 100644
> --- a/t/lib-rebase.sh
> +++ b/t/lib-rebase.sh
> @@ -49,7 +49,7 @@ set_fake_editor () {
>  		case $line in
>  		squash|fixup|edit|reword|drop)
>  			action="$line";;
> -		exec*)
> +		exec*|break)
>  			echo "$line" | sed 's/_/ /g' >> "$1";;
>  		"#")
>  			echo '# comment' >> "$1";;
> diff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh
> index c145dbac38..185a491089 100755
> --- a/t/t3418-rebase-continue.sh
> +++ b/t/t3418-rebase-continue.sh
> @@ -239,5 +239,14 @@ test_rerere_autoupdate -m
>  GIT_SEQUENCE_EDITOR=: && export GIT_SEQUENCE_EDITOR
>  test_rerere_autoupdate -i
>  test_rerere_autoupdate --preserve-merges
> +unset GIT_SEQUENCE_EDITOR
> +
> +test_expect_success 'the todo command "break" works' '
> +	rm -f execed &&
> +	FAKE_LINES="break exec_>execed" git rebase -i HEAD &&
> +	test_path_is_missing execed &&
> +	git rebase --continue &&
> +	test_path_is_file execed
> +'
>  
>  test_done
>
Johannes Schindelin Oct. 12, 2018, 8:35 a.m. UTC | #2
Hi Phillip,

On Thu, 11 Oct 2018, Phillip Wood wrote:

> I think this would be a useful addition to rebase, there's one small
> comment below.
> 
> On 10/10/2018 09:53, Johannes Schindelin via GitGitGadget wrote:
> > From: Johannes Schindelin <johannes.schindelin@gmx.de>
> > 
> > The 'edit' command can be used to cherry-pick a commit and then
> > immediately drop out of the interactive rebase, with exit code 0, to let
> > the user amend the commit, or test it, or look around.
> > 
> > Sometimes this functionality would come in handy *without*
> > cherry-picking a commit, e.g. to interrupt the interactive rebase even
> > before cherry-picking a commit, or immediately after an 'exec' or a
> > 'merge'.
> > 
> > This commit introduces that functionality, as the spanking new 'break'
> > command.
> > 
> > Suggested-by: Stefan Beller <sbeller@google.com>
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > ---
> >  Documentation/git-rebase.txt | 3 +++
> >  rebase-interactive.c         | 1 +
> >  sequencer.c                  | 7 ++++++-
> >  t/lib-rebase.sh              | 2 +-
> >  t/t3418-rebase-continue.sh   | 9 +++++++++
> >  5 files changed, 20 insertions(+), 2 deletions(-)
> > 
> > diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
> > index db2faca73c..5bed1da36b 100644
> > --- a/Documentation/git-rebase.txt
> > +++ b/Documentation/git-rebase.txt
> > @@ -561,6 +561,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.
> >  
> > +To interrupt the rebase (just like an "edit" command would do, but without
> > +cherry-picking any commit first), use the "break" command.
> > +
> >  If you just want to edit the commit message for a commit, replace the
> >  command "pick" with the command "reword".
> >  
> > diff --git a/rebase-interactive.c b/rebase-interactive.c
> > index 0f4119cbae..78f3263fc1 100644
> > --- a/rebase-interactive.c
> > +++ b/rebase-interactive.c
> > @@ -14,6 +14,7 @@ void append_todo_help(unsigned edit_todo, unsigned keep_empty,
> >  "s, squash <commit> = use commit, but meld into previous commit\n"
> >  "f, fixup <commit> = like \"squash\", but discard this commit's log message\n"
> >  "x, exec <command> = run command (the rest of the line) using shell\n"
> > +"b, break = stop here (continue rebase later with 'git rebase --continue')\n"
> >  "d, drop <commit> = remove commit\n"
> >  "l, label <label> = label current HEAD with a name\n"
> >  "t, reset <label> = reset HEAD to a label\n"
> > diff --git a/sequencer.c b/sequencer.c
> > index 8dd6db5a01..b209f8af46 100644
> > --- a/sequencer.c
> > +++ b/sequencer.c
> > @@ -1416,6 +1416,7 @@ enum todo_command {
> >  	TODO_SQUASH,
> >  	/* commands that do something else than handling a single commit */
> >  	TODO_EXEC,
> > +	TODO_BREAK,
> >  	TODO_LABEL,
> >  	TODO_RESET,
> >  	TODO_MERGE,
> > @@ -1437,6 +1438,7 @@ static struct {
> >  	{ 'f', "fixup" },
> >  	{ 's', "squash" },
> >  	{ 'x', "exec" },
> > +	{ 'b', "break" },
> >  	{ 'l', "label" },
> >  	{ 't', "reset" },
> >  	{ 'm', "merge" },
> > @@ -1964,7 +1966,7 @@ static int parse_insn_line(struct todo_item *item, const char *bol, char *eol)
> >  	padding = strspn(bol, " \t");
> >  	bol += padding;
> >  
> > -	if (item->command == TODO_NOOP) {
> > +	if (item->command == TODO_NOOP || item->command == TODO_BREAK) {
> >  		if (bol != eol)
> >  			return error(_("%s does not accept arguments: '%s'"),
> >  				     command_to_string(item->command), bol);
> > @@ -3293,6 +3295,9 @@ static int pick_commits(struct todo_list *todo_list, struct replay_opts *opts)
> >  			unlink(rebase_path_stopped_sha());
> >  			unlink(rebase_path_amend());
> >  			delete_ref(NULL, "REBASE_HEAD", NULL, REF_NO_DEREF);
> > +
> > +			if (item->command == TODO_BREAK)
> > +				break;
> 
> Normally when rebase stops it prints a message to say where it has got
> to and how to continue, it might be useful to do the same here

That's a very valid point. I'll think of something.

Ciao,
Dscho

> 
> Best Wishes
> 
> Phillip
> 
> >  		}
> >  		if (item->command <= TODO_SQUASH) {
> >  			if (is_rebase_i(opts))
> > diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh
> > index 25a77ee5cb..584604ee63 100644
> > --- a/t/lib-rebase.sh
> > +++ b/t/lib-rebase.sh
> > @@ -49,7 +49,7 @@ set_fake_editor () {
> >  		case $line in
> >  		squash|fixup|edit|reword|drop)
> >  			action="$line";;
> > -		exec*)
> > +		exec*|break)
> >  			echo "$line" | sed 's/_/ /g' >> "$1";;
> >  		"#")
> >  			echo '# comment' >> "$1";;
> > diff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh
> > index c145dbac38..185a491089 100755
> > --- a/t/t3418-rebase-continue.sh
> > +++ b/t/t3418-rebase-continue.sh
> > @@ -239,5 +239,14 @@ test_rerere_autoupdate -m
> >  GIT_SEQUENCE_EDITOR=: && export GIT_SEQUENCE_EDITOR
> >  test_rerere_autoupdate -i
> >  test_rerere_autoupdate --preserve-merges
> > +unset GIT_SEQUENCE_EDITOR
> > +
> > +test_expect_success 'the todo command "break" works' '
> > +	rm -f execed &&
> > +	FAKE_LINES="break exec_>execed" git rebase -i HEAD &&
> > +	test_path_is_missing execed &&
> > +	git rebase --continue &&
> > +	test_path_is_file execed
> > +'
> >  
> >  test_done
> > 
> 
>
Phillip Wood Oct. 12, 2018, 11:09 a.m. UTC | #3
Hi Johannes
On 12/10/2018 09:35, Johannes Schindelin wrote:
> Hi Phillip,
> 
> On Thu, 11 Oct 2018, Phillip Wood wrote:
> 
>> I think this would be a useful addition to rebase, there's one small
>> comment below.
>>
>> On 10/10/2018 09:53, Johannes Schindelin via GitGitGadget wrote:
>>> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>>>
>>> The 'edit' command can be used to cherry-pick a commit and then
>>> immediately drop out of the interactive rebase, with exit code 0, to let
>>> the user amend the commit, or test it, or look around.
>>>
>>> Sometimes this functionality would come in handy *without*
>>> cherry-picking a commit, e.g. to interrupt the interactive rebase even
>>> before cherry-picking a commit, or immediately after an 'exec' or a
>>> 'merge'.
>>>
>>> This commit introduces that functionality, as the spanking new 'break'
>>> command.
>>>
>>> Suggested-by: Stefan Beller <sbeller@google.com>
>>> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
>>> ---

>>> diff --git a/sequencer.c b/sequencer.c
>>> index 8dd6db5a01..b209f8af46 100644
>>> --- a/sequencer.c
>>> +++ b/sequencer.c

>>> @@ -3293,6 +3295,9 @@ static int pick_commits(struct todo_list *todo_list, struct replay_opts *opts)
>>>  			unlink(rebase_path_stopped_sha());
>>>  			unlink(rebase_path_amend());
>>>  			delete_ref(NULL, "REBASE_HEAD", NULL, REF_NO_DEREF);
>>> +
>>> +			if (item->command == TODO_BREAK)
>>> +				break;
>>
>> Normally when rebase stops it prints a message to say where it has got
>> to and how to continue, it might be useful to do the same here
> 
> That's a very valid point. I'll think of something.

I'm not sure what gets left on the screen from the previous picks but
something to say what the last pick/exec was and maybe what the current
HEAD is would be useful I think. One thing has just occurred to me -
what does git status say (and does it still work - I'm not sure how much
parsing it does on the todo and done files) if it is run while rebase is
stopped on a break command?

Best Wishes

Phillip
Johannes Schindelin Oct. 12, 2018, 11:24 a.m. UTC | #4
Hi Phillip,

On Fri, 12 Oct 2018, Phillip Wood wrote:

> Hi Johannes
> On 12/10/2018 09:35, Johannes Schindelin wrote:
> > Hi Phillip,
> > 
> > On Thu, 11 Oct 2018, Phillip Wood wrote:
> > 
> >> I think this would be a useful addition to rebase, there's one small
> >> comment below.
> >>
> >> On 10/10/2018 09:53, Johannes Schindelin via GitGitGadget wrote:
> >>> From: Johannes Schindelin <johannes.schindelin@gmx.de>
> >>>
> >>> The 'edit' command can be used to cherry-pick a commit and then
> >>> immediately drop out of the interactive rebase, with exit code 0, to let
> >>> the user amend the commit, or test it, or look around.
> >>>
> >>> Sometimes this functionality would come in handy *without*
> >>> cherry-picking a commit, e.g. to interrupt the interactive rebase even
> >>> before cherry-picking a commit, or immediately after an 'exec' or a
> >>> 'merge'.
> >>>
> >>> This commit introduces that functionality, as the spanking new 'break'
> >>> command.
> >>>
> >>> Suggested-by: Stefan Beller <sbeller@google.com>
> >>> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> >>> ---
> 
> >>> diff --git a/sequencer.c b/sequencer.c
> >>> index 8dd6db5a01..b209f8af46 100644
> >>> --- a/sequencer.c
> >>> +++ b/sequencer.c
> 
> >>> @@ -3293,6 +3295,9 @@ static int pick_commits(struct todo_list *todo_list, struct replay_opts *opts)
> >>>  			unlink(rebase_path_stopped_sha());
> >>>  			unlink(rebase_path_amend());
> >>>  			delete_ref(NULL, "REBASE_HEAD", NULL, REF_NO_DEREF);
> >>> +
> >>> +			if (item->command == TODO_BREAK)
> >>> +				break;
> >>
> >> Normally when rebase stops it prints a message to say where it has got
> >> to and how to continue, it might be useful to do the same here
> > 
> > That's a very valid point. I'll think of something.
> 
> I'm not sure what gets left on the screen from the previous picks but
> something to say what the last pick/exec was and maybe what the current
> HEAD is would be useful I think.

I first wanted to do that, imitating the "Stopped at <hex>... <oneline>"
of an `edit` command, but it *is* now possible that we are on an unborn
branch... which will look a bit funny, I guess, as we construct a very
special place-holder commit to be HEAD in that case.

> One thing has just occurred to me - what does git status say (and does
> it still work - I'm not sure how much parsing it does on the todo and
> done files) if it is run while rebase is stopped on a break command?

I don't think it says anything special, really. It just reports that we're
in the middle of a rebase, listing up to two already-processed and up to
two to-be-processed todo commands.

Ciao,
Dscho

Patch
diff mbox series

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index db2faca73c..5bed1da36b 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -561,6 +561,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.
 
+To interrupt the rebase (just like an "edit" command would do, but without
+cherry-picking any commit first), use the "break" command.
+
 If you just want to edit the commit message for a commit, replace the
 command "pick" with the command "reword".
 
diff --git a/rebase-interactive.c b/rebase-interactive.c
index 0f4119cbae..78f3263fc1 100644
--- a/rebase-interactive.c
+++ b/rebase-interactive.c
@@ -14,6 +14,7 @@  void append_todo_help(unsigned edit_todo, unsigned keep_empty,
 "s, squash <commit> = use commit, but meld into previous commit\n"
 "f, fixup <commit> = like \"squash\", but discard this commit's log message\n"
 "x, exec <command> = run command (the rest of the line) using shell\n"
+"b, break = stop here (continue rebase later with 'git rebase --continue')\n"
 "d, drop <commit> = remove commit\n"
 "l, label <label> = label current HEAD with a name\n"
 "t, reset <label> = reset HEAD to a label\n"
diff --git a/sequencer.c b/sequencer.c
index 8dd6db5a01..b209f8af46 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1416,6 +1416,7 @@  enum todo_command {
 	TODO_SQUASH,
 	/* commands that do something else than handling a single commit */
 	TODO_EXEC,
+	TODO_BREAK,
 	TODO_LABEL,
 	TODO_RESET,
 	TODO_MERGE,
@@ -1437,6 +1438,7 @@  static struct {
 	{ 'f', "fixup" },
 	{ 's', "squash" },
 	{ 'x', "exec" },
+	{ 'b', "break" },
 	{ 'l', "label" },
 	{ 't', "reset" },
 	{ 'm', "merge" },
@@ -1964,7 +1966,7 @@  static int parse_insn_line(struct todo_item *item, const char *bol, char *eol)
 	padding = strspn(bol, " \t");
 	bol += padding;
 
-	if (item->command == TODO_NOOP) {
+	if (item->command == TODO_NOOP || item->command == TODO_BREAK) {
 		if (bol != eol)
 			return error(_("%s does not accept arguments: '%s'"),
 				     command_to_string(item->command), bol);
@@ -3293,6 +3295,9 @@  static int pick_commits(struct todo_list *todo_list, struct replay_opts *opts)
 			unlink(rebase_path_stopped_sha());
 			unlink(rebase_path_amend());
 			delete_ref(NULL, "REBASE_HEAD", NULL, REF_NO_DEREF);
+
+			if (item->command == TODO_BREAK)
+				break;
 		}
 		if (item->command <= TODO_SQUASH) {
 			if (is_rebase_i(opts))
diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh
index 25a77ee5cb..584604ee63 100644
--- a/t/lib-rebase.sh
+++ b/t/lib-rebase.sh
@@ -49,7 +49,7 @@  set_fake_editor () {
 		case $line in
 		squash|fixup|edit|reword|drop)
 			action="$line";;
-		exec*)
+		exec*|break)
 			echo "$line" | sed 's/_/ /g' >> "$1";;
 		"#")
 			echo '# comment' >> "$1";;
diff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh
index c145dbac38..185a491089 100755
--- a/t/t3418-rebase-continue.sh
+++ b/t/t3418-rebase-continue.sh
@@ -239,5 +239,14 @@  test_rerere_autoupdate -m
 GIT_SEQUENCE_EDITOR=: && export GIT_SEQUENCE_EDITOR
 test_rerere_autoupdate -i
 test_rerere_autoupdate --preserve-merges
+unset GIT_SEQUENCE_EDITOR
+
+test_expect_success 'the todo command "break" works' '
+	rm -f execed &&
+	FAKE_LINES="break exec_>execed" git rebase -i HEAD &&
+	test_path_is_missing execed &&
+	git rebase --continue &&
+	test_path_is_file execed
+'
 
 test_done