Re: [PATCH] rebase -i: do not update "done" when rescheduling command

[Phillip Wood <phillip.wood123@gmail.com>]

Hi Dscho

Sorry for not replying sooner.

On 27/03/2023 08:04, Johannes Schindelin wrote:
> Hi Phillip,
> 
> On Sun, 19 Mar 2023, Phillip Wood via GitGitGadget wrote:
> 
>> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>>
>> As the sequencer executes todo commands it appends them to
>> .git/rebase-merge/done. This file is used by "git status" to show the
>> recently executed commands. Unfortunately when a command is rescheduled
>> the command preceding it is erroneously appended to the "done" file.
>> This means that when rebase stops after rescheduling "pick B" the "done"
>> file contains
>>
>> 	pick A
>> 	pick B
>> 	pick A
>>
>> instead of
>>
>> 	pick A
>> 	pick B
>>
>> Fix this by not updating the "done" file when adding a rescheduled
>> command back into the "todo" file. A couple of the existing tests are
>> modified to improve their coverage as none of them trigger this bug or
>> check the "done" file.
> 
> I am purposefully not yet focusing on the patch, as I have a concern about
> the reasoning above.
> 
> When a command fails that needs to be rescheduled, I actually _like_ that
> there is a record in `done` about said command. It is very much like a
> `pick` that failed (but was not rescheduled) and was then `--skip`ed: it
> still shows up on `done`.

We still do that after this patch. What changes is that when "pick B" 
fails we don't add "pick A" to the "done" file when "pick B" is added 
back into "git-rebase-todo"

> I do understand the concern that the rescheduled command now shows up in
> both `done` and `git-rebase-todo` (which is very different from the failed
> `pick` command that would show up _only_ in `git-rebase-todo`). So maybe
> we can find some compromise, e.g. adding a commented-out line to `done` à
> la:
> 
> 	# rescheduled: pick A
> 
> What do you think?

If a commit is rescheduled we still end up with multiple entries in the 
"done". In the example above if "pick B" fails the first time it is 
executed and then succeeds on the second attempt "done" will contain

	pick A
	pick B
	pick B

It might be nice to mark it as rescheduled as you suggest but this 
series focuses on removing the incorrect entry from the "done" file, not 
de-duplicating the "done" entities when a command fails.

Best Wishes

Phillip

> Ciao,
> Johannes