| Message ID | fff6fec5-88c9-4125-bf51-5e96e34bf1f6@kdbg.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | rebase -i: introduce the 'break' command | expand |
Johannes Sixt <j6t@kdbg.org> writes: > The sequencer instruction 'b', short for 'break', is rejected: > > error: invalid line 2: b > > The reason is that the parser expects all short commands to have > an argument. Permit short commands without arguments. > > Signed-off-by: Johannes Sixt <j6t@kdbg.org> > --- > I'll send a another patch in a moment that tests all short > sequencer commands, but it is independent from this topic. > > sequencer.c | 3 ++- > t/lib-rebase.sh | 2 +- > t/t3418-rebase-continue.sh | 4 +++- > 3 files changed, 6 insertions(+), 3 deletions(-) > > diff --git a/sequencer.c b/sequencer.c > index ee3961ec63..3107f59ea7 100644 > --- a/sequencer.c > +++ b/sequencer.c > @@ -1954,7 +1954,8 @@ static int parse_insn_line(struct todo_item *item, const char *bol, char *eol) > if (skip_prefix(bol, todo_command_info[i].str, &bol)) { > item->command = i; > break; > - } else if (bol[1] == ' ' && *bol == todo_command_info[i].c) { > + } else if ((bol + 1 == eol || bol[1] == ' ') && > + *bol == todo_command_info[i].c) { > bol++; > item->command = i; > break; > diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh > index 584604ee63..86572438ec 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*|break) > + exec*|break|b) > echo "$line" | sed 's/_/ /g' >> "$1";; > "#") > echo '# comment' >> "$1";; > diff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh > index 185a491089..b282505aac 100755 > --- a/t/t3418-rebase-continue.sh > +++ b/t/t3418-rebase-continue.sh > @@ -243,7 +243,9 @@ unset GIT_SEQUENCE_EDITOR > > test_expect_success 'the todo command "break" works' ' > rm -f execed && > - FAKE_LINES="break exec_>execed" git rebase -i HEAD && > + FAKE_LINES="break b exec_>execed" git rebase -i HEAD && > + test_path_is_missing execed && When first 'break' hits, "git rebase -i" shouldn't have exited with non-zero, and we get to see if execed is there (it shouldn't exist yet). > + git rebase --continue && And then we continue, to hit the next 'b', which shouldn't barf, either, and then > test_path_is_missing execed && we make sure execed is not yet there, before continuing out of that that 'b'roken state > git rebase --continue && and then we'll hit the exec to create that file. > test_path_is_file execed Makes sense. Thanks.
Hi Hannes, On Thu, 25 Oct 2018, Johannes Sixt wrote: > The sequencer instruction 'b', short for 'break', is rejected: > > error: invalid line 2: b > > The reason is that the parser expects all short commands to have > an argument. Permit short commands without arguments. > > Signed-off-by: Johannes Sixt <j6t@kdbg.org> > --- ACK. Thanks for fixing this, Dscho > I'll send a another patch in a moment that tests all short > sequencer commands, but it is independent from this topic. > > sequencer.c | 3 ++- > t/lib-rebase.sh | 2 +- > t/t3418-rebase-continue.sh | 4 +++- > 3 files changed, 6 insertions(+), 3 deletions(-) > > diff --git a/sequencer.c b/sequencer.c > index ee3961ec63..3107f59ea7 100644 > --- a/sequencer.c > +++ b/sequencer.c > @@ -1954,7 +1954,8 @@ static int parse_insn_line(struct todo_item *item, const char *bol, char *eol) > if (skip_prefix(bol, todo_command_info[i].str, &bol)) { > item->command = i; > break; > - } else if (bol[1] == ' ' && *bol == todo_command_info[i].c) { > + } else if ((bol + 1 == eol || bol[1] == ' ') && > + *bol == todo_command_info[i].c) { > bol++; > item->command = i; > break; > diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh > index 584604ee63..86572438ec 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*|break) > + exec*|break|b) > echo "$line" | sed 's/_/ /g' >> "$1";; > "#") > echo '# comment' >> "$1";; > diff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh > index 185a491089..b282505aac 100755 > --- a/t/t3418-rebase-continue.sh > +++ b/t/t3418-rebase-continue.sh > @@ -243,7 +243,9 @@ unset GIT_SEQUENCE_EDITOR > > test_expect_success 'the todo command "break" works' ' > rm -f execed && > - FAKE_LINES="break exec_>execed" git rebase -i HEAD && > + FAKE_LINES="break b exec_>execed" git rebase -i HEAD && > + test_path_is_missing execed && > + git rebase --continue && > test_path_is_missing execed && > git rebase --continue && > test_path_is_file execed > -- > 2.19.1.406.g1aa3f475f3 >
diff --git a/sequencer.c b/sequencer.c index ee3961ec63..3107f59ea7 100644 --- a/sequencer.c +++ b/sequencer.c @@ -1954,7 +1954,8 @@ static int parse_insn_line(struct todo_item *item, const char *bol, char *eol) if (skip_prefix(bol, todo_command_info[i].str, &bol)) { item->command = i; break; - } else if (bol[1] == ' ' && *bol == todo_command_info[i].c) { + } else if ((bol + 1 == eol || bol[1] == ' ') && + *bol == todo_command_info[i].c) { bol++; item->command = i; break; diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh index 584604ee63..86572438ec 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*|break) + exec*|break|b) echo "$line" | sed 's/_/ /g' >> "$1";; "#") echo '# comment' >> "$1";; diff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh index 185a491089..b282505aac 100755 --- a/t/t3418-rebase-continue.sh +++ b/t/t3418-rebase-continue.sh @@ -243,7 +243,9 @@ unset GIT_SEQUENCE_EDITOR test_expect_success 'the todo command "break" works' ' rm -f execed && - FAKE_LINES="break exec_>execed" git rebase -i HEAD && + FAKE_LINES="break b exec_>execed" git rebase -i HEAD && + test_path_is_missing execed && + git rebase --continue && test_path_is_missing execed && git rebase --continue && test_path_is_file execed
The sequencer instruction 'b', short for 'break', is rejected: error: invalid line 2: b The reason is that the parser expects all short commands to have an argument. Permit short commands without arguments. Signed-off-by: Johannes Sixt <j6t@kdbg.org> --- I'll send a another patch in a moment that tests all short sequencer commands, but it is independent from this topic. sequencer.c | 3 ++- t/lib-rebase.sh | 2 +- t/t3418-rebase-continue.sh | 4 +++- 3 files changed, 6 insertions(+), 3 deletions(-)