Changes from v4:
- Put Phillip's suggested explanation in the commit message
- Check for truncation both after the first `git rebase` and after
`git rebase --edit-todo`
Thanks to Phillip and Junio for your feedback.
Alex Henrie (1):
sequencer: finish parsing the todo list despite an invalid first line
sequencer.c | 2 +-
t/t3404-rebase-interactive.sh | 27 +++++++++++++++++++++++++++
2 files changed, 28 insertions(+), 1 deletion(-)
Range-diff against v4:
1: f6fcdcd9a9 ! 1: 6fbe4fd3e6 sequencer: finish parsing the todo list despite an invalid first line
@@ Metadata
## Commit message ##
sequencer: finish parsing the todo list despite an invalid first line
- ddb81e5072 (rebase-interactive: use todo_list_write_to_file() in
- edit_todo_list(), 2019-03-05) made edit_todo_list more efficient by
- replacing transform_todo_file with todo_list_parse_insn_buffer.
- Unfortunately, that innocuous change caused a regression because
- todo_list_parse_insn_buffer would stop parsing after encountering an
- invalid 'fixup' line. If the user accidentally made the first line a
- 'fixup' and tried to recover from their mistake with `git rebase
- --edit-todo`, all of the commands after the first would be lost.
+ Before the todo list is edited it is rewritten to shorten the OIDs of
+ the commits being picked and to append advice about editing the list.
+ The exact advice depends on whether the todo list is being edited for
+ the first time or not. After the todo list has been edited it is
+ rewritten to lengthen the OIDs of the commits being picked and to remove
+ the advice. If the edited list cannot be parsed then this last step is
+ skipped.
- To avoid throwing away important parts of the todo list, change
- todo_list_parse_insn_buffer to keep going and not return early on error.
+ Prior to db81e50724 (rebase-interactive: use todo_list_write_to_file()
+ in edit_todo_list(), 2019-03-05) if the existing todo list could not be
+ parsed then the initial rewrite was skipped as well. This had the
+ unfortunate consequence that if the list could not be parsed after the
+ initial edit the advice given to the user was wrong when they re-edited
+ the list. This change relied on todo_list_parse_insn_buffer() returning
+ the whole todo list even when it cannot be parsed. Unfortunately if the
+ list starts with a "fixup" command then it will be truncated and the
+ remaining lines are lost. Fix this by continuing to parse after an
+ initial "fixup" commit as we do when we see any other invalid line.
Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
@@ t/t3404-rebase-interactive.sh: test_expect_success 'static check of bad command'
+test_expect_success 'the first command cannot be a fixup' '
+ rebase_setup_and_clean fixup-first &&
+ (
-+ set_fake_editor &&
-+ test_must_fail env FAKE_LINES="fixup 1 2 3 4" \
-+ git rebase -i HEAD~4 2>actual &&
++ cat >orig <<-EOF &&
++ fixup $(git log -1 --format="%h %s" B)
++ pick $(git log -1 --format="%h %s" C)
++ EOF
++
++ (
++ set_replace_editor orig &&
++ test_must_fail git rebase -i A 2>actual
++ ) &&
+ grep "cannot .fixup. without a previous commit" actual &&
+ grep "You can fix this with .git rebase --edit-todo.." actual &&
-+ grep -v "^#" .git/rebase-merge/git-rebase-todo >orig &&
++ # verify that the todo list has not been truncated
++ grep -v "^#" .git/rebase-merge/git-rebase-todo >actual &&
++ test_cmp orig actual &&
++
+ test_must_fail git rebase --edit-todo 2>actual &&
+ grep "cannot .fixup. without a previous commit" actual &&
+ grep "You can fix this with .git rebase --edit-todo.." actual &&
++ # verify that the todo list has not been truncated
+ grep -v "^#" .git/rebase-merge/git-rebase-todo >actual &&
-+ # check that --edit-todo did not lose any of the todo list
+ test_cmp orig actual
+ )
+'Alex Henrie <alexhenrie24@gmail.com>