Re: [PATCH] sequencer: finish parsing the todo list despite an invalid first line
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jul 21, 2023, 15:21 UTC
- Message-ID
- <xmqqpm4lwasj.fsf@gitster.g>
- In-Reply-To
- <d6e54535-79a3-2d2c-3152-4cabc5bbd9b8@gmail.com>
Phillip Wood <phillip.wood123@gmail.com> writes:
Show 29 quoted lines
> 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.