Re: [PATCH] sequencer: finish parsing the todo list despite an invalid first line

[Junio C Hamano <gitster@pobox.com>]

Phillip Wood <phillip.wood123@gmail.com> writes:

> On 21/07/2023 10:31, Phillip Wood wrote:
>> That's fine but the commit message should explain that decision and
>> clarify why ddb81e5072 caused the regression
>
> Maybe something like
>
> 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.
>
> Best Wishes
>
> Phillip

That does sounds a lot easier to understand as an explanation for
the reason why this change is necessary and sufficient.

Thanks for reviewing.