Re: [PATCH v5 1/1] sequencer: finish parsing the todo list despite an invalid first line
- From
Alex Henrie <alexhenrie24@gmail.com>
- Date
- Jul 24, 2023, 15:26 UTC
- Message-ID
- <CAMMLpeTBBS7FExevcvCWut8wFbcDSDBhUUq+tCaXfOPiY+3GXA@mail.gmail.com>
- In-Reply-To
- <0d1c5bfd-3ae5-83f0-a333-bbb8510a973a@gmail.com>
On Mon, Jul 24, 2023 at 4:02 AM Phillip Wood <phillip.wood123@gmail.com> wrote:
Show 23 quoted lines
> On 22/07/2023 22:28, Alex Henrie wrote: > > 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. > > This version looks great apart from the test being run in an unnecessary > subshell which looks like it got left in from the last version. Junio > might be able to correct that when he applies the patch.
I think I see what you mean now: Because this test never performs a successful rebase, rebase_setup_and_clean is overkill. I can send a v6 tonight that uses 'test_when_finished "git rebase --abort"' instead.
Thanks,
-Alex