[PATCH v5 0/1] sequencer: finish parsing the todo list despite an invalid first line

[Alex Henrie <alexhenrie24@gmail.com>]

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
     +	)
     +'
-- 
2.41.0