| Message ID | c74e02c4b643652d43108c1a5a675df0fae5f059.1539161632.git.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | rebase -i: introduce the 'break' command | expand |
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 >
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 > > > >
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
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
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