sequencer: finish parsing the todo list despite an invalid first line

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. 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> [jc: removed an apparently unneeded subshell around the test body] Signed-off-by: Junio C Hamano <gitster@pobox.com>

Alex Henrie committed Jul 22, 2023 at 15:28 UTC 9645a087c2e848b8f48d03a3ae3222c5d5aa8261
2 files changed +27 -1
sequencer.c
+1 -1
@@ -2680,7 +2680,7 @@ int todo_list_parse_insn_buffer(struct repository *r, char *buf,
2680 if (fixup_okay)
2681 ; /* do nothing */
2682 else if (is_fixup(item->command))
2683 - return error(_("cannot '%s' without a previous commit"),
2683 + res = error(_("cannot '%s' without a previous commit"),
2684 command_to_string(item->command));
2685 else if (!is_noop(item->command))
2686 fixup_okay = 1;
t/t3404-rebase-interactive.sh
+26
@@ -1596,6 +1596,32 @@ test_expect_success 'static check of bad command' '
1596 test C = $(git cat-file commit HEAD^ | sed -ne \$p)
1597 '
1598
1599 +test_expect_success 'the first command cannot be a fixup' '
1600 + rebase_setup_and_clean fixup-first &&
1601 +
1602 + cat >orig <<-EOF &&
1603 + fixup $(git log -1 --format="%h %s" B)
1604 + pick $(git log -1 --format="%h %s" C)
1605 + EOF
1606 +
1607 + (
1608 + set_replace_editor orig &&
1609 + test_must_fail git rebase -i A 2>actual
1610 + ) &&
1611 + grep "cannot .fixup. without a previous commit" actual &&
1612 + grep "You can fix this with .git rebase --edit-todo.." actual &&
1613 + # verify that the todo list has not been truncated
1614 + grep -v "^#" .git/rebase-merge/git-rebase-todo >actual &&
1615 + test_cmp orig actual &&
1616 +
1617 + test_must_fail git rebase --edit-todo 2>actual &&
1618 + grep "cannot .fixup. without a previous commit" actual &&
1619 + grep "You can fix this with .git rebase --edit-todo.." actual &&
1620 + # verify that the todo list has not been truncated
1621 + grep -v "^#" .git/rebase-merge/git-rebase-todo >actual &&
1622 + test_cmp orig actual
1623 +'
1624 +
1625 test_expect_success 'tabs and spaces are accepted in the todolist' '
1626 rebase_setup_and_clean indented-comment &&
1627 write_script add-indent.sh <<-\EOF &&