Aborting git rebase --edit-todo

> When doing a commit or choosing what to do for an interactive rebase
> one can just wipe the whole content of the editor, save and close to
> abort the action.
> While doing a `git rebase --edit-todo` I came to the conclusion that I
> would like to abort the edit and did the same. The final `git rebase
> --continue` got me rid of the rest of the commits...
> (Fortunately the "missing" commits could be rescued by looking into
> `.git/logs/HEAD` so thumbs up for that. )
> Unfortunately the behaviour of `--edit-todo` was a bit surprising and
> somehow doesn't feel consistent with the other actions involving an
> editor.
>
> Can this be considered a bug?

> > Now we have the "drop" verb, the latter interpretation becomes
> > possible without making it impossible for the user to express the
> > former.
>
> and for people that would like to enforce the use of the drop verb
> there is configuration that prevents deleted lines to "silently"
> dropping commits since 5a5445d878 (rebase-interactive: warn if commit
> is dropped with `rebase --edit-todo', 2020-01-28) :
>
>   rebase.missingCommitsCheck
>

> AFAIK the correct "signal" to abort is to instruct your editor to exit
> with non zero (ex: in vi using <esc>:cq), but agree it could be
> confusing or "inconsistent" and might be worth adding it a message at
> the footer
>

> It is rather unusual (or almost always wrong) to have a totally
> empty commit log or initial todo list, so it is understandable for
> Git in these situations to stop without doing anything further.
>
> There is no other sensible interpretations of what you are telling
> Git to you by returning an empty buffer---it is extremely unlikely
> you want to create a commit with no log message (without explicitly
> allowing it with --allow-empty-message, the command is likely to
> fail anyway), and it is extremely unlikely that you wanted to just
> reset the tip of the branch to the --onto commit.
>
> Once an interactive rebase session has started and you are given the
> remainder of the steps to edit and you give an empty buffer back,
> however, there are two possible interpretations that are equally
> sensible, I would think.
>
>  - One is that you are signaling that you are done with the rebase
>    session and all the remaining commits are to be discarded.
>
>  - The other is that you botched editing the todo list, and you wish
>    Git to give you another chance to edit it again.
>
> I think the implementor chose the first interpretation.  The "drop"
> insn is a relatively recent invention, and back when it was missing
> from the vocabulary, I do not think it was possible to say " discard
> all the rest" without emptying the todo list, so that design is
> understandable.
>
> Now we have the "drop" verb, the latter interpretation becomes
> possible without making it impossible for the user to express the
> former.  It might be a good idea to
>
>  (1) save away the original before allowing --edit-todo to edit,
>
>  (2) open the editor, and
>
>  (3) when getting an empty buffer back, go back to step (2) using
>      the back-up made in step (1).
>
> Either way, the todo list editor buffer can have additional comment
> instructing what happens when the buffer is emptied.
>

>
> On Thu, Sep 3, 2020 at 12:36 PM Victor Toni <victor.toni@gmail.com> wrote:
> >
> > is more difficult to recover from. (I'm still thankful for `.git/logs/HEAD`)
>
> another command that might not be that well known and that will always
> recover from a botched merge (or a rebase in this case) :
>
>   $ git reset --hard ORIG_HEAD
>

>> I think the implementor chose the first interpretation.  The "drop"
>> insn is a relatively recent invention, and back when it was missing
>> from the vocabulary, I do not think it was possible to say " discard
>> all the rest" without emptying the todo list, so that design is
>> understandable.
>>
>> Now we have the "drop" verb, the latter interpretation becomes
>> possible without making it impossible for the user to express the
>> former.  It might be a good idea to
>>
>>  (1) save away the original before allowing --edit-todo to edit,
>>
>>  (2) open the editor, and
>>
>>  (3) when getting an empty buffer back, go back to step (2) using
>>      the back-up made in step (1).
>>
>> Either way, the todo list editor buffer can have additional comment
>> instructing what happens when the buffer is emptied.
>>
> Personally I would like to see your approach (1,2,3) implemented
> because it is not destructive. If the user wants to achieve something
> different he can retry.

>
> Victor Toni <victor.toni@gmail.com> writes:
>
> >> I think the implementor chose the first interpretation.  The "drop"
> >> insn is a relatively recent invention, and back when it was missing
> >> from the vocabulary, I do not think it was possible to say " discard
> >> all the rest" without emptying the todo list, so that design is
> >> understandable.
> >>
> >> Now we have the "drop" verb, the latter interpretation becomes
> >> possible without making it impossible for the user to express the
> >> former.  It might be a good idea to
> >>
> >>  (1) save away the original before allowing --edit-todo to edit,
> >>
> >>  (2) open the editor, and
> >>
> >>  (3) when getting an empty buffer back, go back to step (2) using
> >>      the back-up made in step (1).
> >>
> >> Either way, the todo list editor buffer can have additional comment
> >> instructing what happens when the buffer is emptied.
> >>
> > Personally I would like to see your approach (1,2,3) implemented
> > because it is not destructive. If the user wants to achieve something
> > different he can retry.
>
> Obviously I agree that the approach would be nicer than the status
> quo.  It would not be as trivial as a microproject, but would be a
> good bite-sized starter-task for those aspiring developers who want
> to dip their toes in the water to start hacking on the codebase ;-)
>

> Junio C Hamano <gitster@pobox.com> writes:
> >
> > Victor Toni <victor.toni@gmail.com> writes:
> >
> > >> I think the implementor chose the first interpretation.  The "drop"
> > >> insn is a relatively recent invention, and back when it was missing
> > >> from the vocabulary, I do not think it was possible to say " discard
> > >> all the rest" without emptying the todo list, so that design is
> > >> understandable.
> > >>
> > >> Now we have the "drop" verb, the latter interpretation becomes
> > >> possible without making it impossible for the user to express the
> > >> former.  It might be a good idea to
> > >>
> > >>  (1) save away the original before allowing --edit-todo to edit,
> > >>
> > >>  (2) open the editor, and
> > >>
> > >>  (3) when getting an empty buffer back, go back to step (2) using
> > >>      the back-up made in step (1).
> > >>
> > >> Either way, the todo list editor buffer can have additional comment
> > >> instructing what happens when the buffer is emptied.
> > >>
> > > Personally I would like to see your approach (1,2,3) implemented
> > > because it is not destructive. If the user wants to achieve something
> > > different he can retry.
> >
> > Obviously I agree that the approach would be nicer than the status
> > quo.  It would not be as trivial as a microproject, but would be a
> > good bite-sized starter-task for those aspiring developers who want
> > to dip their toes in the water to start hacking on the codebase ;-)
> >
> Nice try ;) Speaking of toes ... I'm currently involved in another
> project from tip to toe.
> I would like to come back to your offer sometime next year when I've
> completed the other one.

> Junio C Hamano <gitster@pobox.com> writes:
>>
>> Victor Toni <victor.toni@gmail.com> writes:
>>
>> > Personally I would like to see your approach (1,2,3) implemented
>> > because it is not destructive. If the user wants to achieve something
>> > different he can retry.
>>
>> Obviously I agree that the approach would be nicer than the status
>> quo.  It would not be as trivial as a microproject, but would be a
>> good bite-sized starter-task for those aspiring developers who want
>> to dip their toes in the water to start hacking on the codebase ;-)
>>
> Nice try ;)

> > It is rather unusual (or almost always wrong) to have a totally
> > empty commit log or initial todo list, so it is understandable for
> > Git in these situations to stop without doing anything further.
> >
> > There is no other sensible interpretations of what you are telling
> > Git to you by returning an empty buffer---it is extremely unlikely
> > you want to create a commit with no log message (without explicitly
> > allowing it with --allow-empty-message, the command is likely to
> > fail anyway), and it is extremely unlikely that you wanted to just
> > reset the tip of the branch to the --onto commit.
> >
> > Once an interactive rebase session has started and you are given the
> > remainder of the steps to edit and you give an empty buffer back,
> > however, there are two possible interpretations that are equally
> > sensible, I would think.
> >
> >  - One is that you are signaling that you are done with the rebase
> >    session and all the remaining commits are to be discarded.
> >
> >  - The other is that you botched editing the todo list, and you wish
> >    Git to give you another chance to edit it again.
> >
> > I think the implementor chose the first interpretation.  The "drop"
> > insn is a relatively recent invention, and back when it was missing
> > from the vocabulary, I do not think it was possible to say " discard
> > all the rest" without emptying the todo list, so that design is
> > understandable.
> >
> > Now we have the "drop" verb, the latter interpretation becomes
> > possible without making it impossible for the user to express the
> > former.  It might be a good idea to
> >
> >  (1) save away the original before allowing --edit-todo to edit,

> >
> >  (2) open the editor, and
> >
> >  (3) when getting an empty buffer back, go back to step (2) using
> >      the back-up made in step (1).

> >
> > Either way, the todo list editor buffer can have additional comment
> > instructing what happens when the buffer is emptied.
> >
>
> Personally I would like to see your approach (1,2,3) implemented
> because it is not destructive. If the user wants to achieve something
> different he

> can retry.
> Option / interpretation a)
>
> >  - One is that you are signaling that you are done with the rebase
> >    session and all the remaining commits are to be discarded.
>
> is more difficult to recover from. (I'm still thankful for `.git/logs/HEAD`)

> Victor Toni <victor.toni@gmail.com> writes:
>
> > When doing a commit or choosing what to do for an interactive rebase
> > one can just wipe the whole content of the editor, save and close to
> > abort the action.
> > While doing a `git rebase --edit-todo` I came to the conclusion that I
> > would like to abort the edit and did the same. The final `git rebase
> > --continue` got me rid of the rest of the commits...
> > (Fortunately the "missing" commits could be rescued by looking into
> > `.git/logs/HEAD` so thumbs up for that. )
> > Unfortunately the behaviour of `--edit-todo` was a bit surprising and
> > somehow doesn't feel consistent with the other actions involving an
> > editor.
> >
> > Can this be considered a bug?
>
> It is rather unusual (or almost always wrong) to have a totally
> empty commit log or initial todo list, so it is understandable for
> Git in these situations to stop without doing anything further.
>
> There is no other sensible interpretations of what you are telling
> Git to you by returning an empty buffer---it is extremely unlikely
> you want to create a commit with no log message (without explicitly
> allowing it with --allow-empty-message, the command is likely to
> fail anyway), and it is extremely unlikely that you wanted to just
> reset the tip of the branch to the --onto commit.
>
> Once an interactive rebase session has started and you are given the
> remainder of the steps to edit and you give an empty buffer back,
> however, there are two possible interpretations that are equally
> sensible, I would think.
>
>  - One is that you are signaling that you are done with the rebase
>    session and all the remaining commits are to be discarded.
>
>  - The other is that you botched editing the todo list, and you wish
>    Git to give you another chance to edit it again.
>
> I think the implementor chose the first interpretation.  The "drop"
> insn is a relatively recent invention, and back when it was missing
> from the vocabulary, I do not think it was possible to say " discard
> all the rest" without emptying the todo list, so that design is
> understandable.
>
> Now we have the "drop" verb, the latter interpretation becomes
> possible without making it impossible for the user to express the
> former.  It might be a good idea to
>
>  (1) save away the original before allowing --edit-todo to edit,
>
>  (2) open the editor, and
>
>  (3) when getting an empty buffer back, go back to step (2) using
>      the back-up made in step (1).
>
> Either way, the todo list editor buffer can have additional comment
> instructing what happens when the buffer is emptied.
>
> I have no strong opinion on this one myself.  Deferring to Dscho,
> who may have a lot more to say on the design issue around this
> feature than I do.