Hi Phillip, On Thu, 3 Aug 2023, Phillip Wood wrote: > On 27/03/2023 08:04, Johannes Schindelin wrote: > > > > On Sun, 19 Mar 2023, Phillip Wood via GitGitGadget wrote: > > > > > From: Phillip Wood <phillip.wood@xxxxxxxxxxxxx> > > > > > > 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. After having had plenty of time to let this issue simmer in the back of my head, and after reviewing the latest iteration of the patch series, I am no longer concerned, as it now sounds more logical to me, too, that rescheduled commands don't show up in `done`. Thank you, Johannes
