Re: [PATCH 7/7] sequencer: Remove sequencer state after final commit

[Junio C Hamano <gitster@pobox.com>]

Junio C Hamano <gitster@pobox.com> writes:

> Jonathan Nieder <jrnieder@gmail.com> writes:
>
>> I believe it is meant to support command sequences such as these:
>>
>> 1.
>> 	git cherry-pick foo; # has conflicts
>> 	... resolve conflicts and "git add" the resolved files ...
>> 	git commit
>> 	git cherry-pick bar
>
> Why does a single commit "cherry-pick foo" leave any sequencer state that
> may interfere with the latter to begin with? Isn't that already a bug?
>
>> 2.
>> 	git cherry-pick foo bar; # has conflicts applying "bar"
>> 	... resolve ...
>> 	git commit
>> 	git cherry-pick baz
>>
>> Those were intuitive things to do before the sequencer existed, and if
>> I understand correctly, d3f4628e was intended to support people and
>> scripts (such as the test suite) that have these commands wired into
>> their fingers.
>
> Given that the latter was broken when "foo" stopped with conficts (it lost
> "bar" altogether anyway), I am not worried about it, and I do not care
> much about anybody who had wired such multi-pick into scripts or fingers,
> either.
>
> IOW, I do not necessarily agree with your "those were intuitive"
> assertion.

After sleeping on this, here are my random thoughts on this topic.

 - The sequencing flow the current "rebase -i" implements when resuming a
   controlled stop (i.e. "edit" or "reword"), even as the last step of the
   insn sheet, feels like the right thing. The actual tweaking of the
   commit done by "commit --amend" is oblivious to the sequencing state,
   and resuming is controlled by "rebase --continue".

 - The case to resume an unexpected stop (i.e. "pick" that causes conflict
   or "rebase" without "-i") also feels right. The user fixes up and marks
   things as "done" with "add/rm", and tell the stopped command that it can
   now continue with "rebase --continue". The same comment applies for
   "am".

 - Even before we started talking about the sequencer, the workflow to deal
   with conflicted "revert" or "cherry-pick" was awkward. From the end
   user's point of view, the operation the end user wanted to perform was
   "revert" (or "cherry-pick"), but we tell the user to fix things up, mark
   them as "done" with "add/rm", and commit the result themselves. We could
   have made the UI more consistent by introducing "revert --continue".

 - In a sense, the last point got worse by a recent change to make
   "commit" notice CHERRY_PICK_HEAD. This was modeled the workflow to
   conclude a conflicted "merge", that noticed "MERGE_HEAD" etc., but from
   the UI point of view, I think it was a mistake. Back when "merge" was
   the only command that needs post-clean-up by the user, saying "when
   merge stops due to conflict, you conclude it with commit, but
   everything else you conclude (or continue) with --continue" was
   reasonable, but now we have to say "after conflicted merge, revert and
   cherry-pick, you conclude it with commit; use --continue for everything
   else".

So I think that questions that affect us longer term are:

 - Does it make sense to keep this two quite different ways to resume an
   interrupted operation? Some saying "The operation you started was Foo
   but you need to conclude it with 'commit'", others saying "When Bar
   stops in the middle, you say 'Bar --continue'"?

 - If so, which camp should the sequencer-based commands fall into?

 - If not, shouldn't we be unifying the user experience to one of them,
   with backward compatibility escape hatches?

I think what d3f4628e tried to do may make sense _if_ we want to make
"commit" the way to conclude or continue sequencer-based commands. If we
really wanted to go that route, however, it would make it easier if
"commit" were the _only_ way to deal with all the "stopped because the
user needs to resolve conflicts or because the user told us to pause"
situations. For example, when "rebase -i" stops (either due to an "edit"
or a conflicted "pick"), after the user tweaks the tree and says "git
commit [--amend]" to reword, "rebase --continue" after that shouldn't be
necessary. If we are making "commit" aware of the sequencer status, it
should already know that the next thing after that invocation of "commit"
is to resume the sequencing.

But I do not think that is the direction we would want to go for two
reasons. There is a (complicated) workflow to split an commit during
"rebase -i" that does _not_ want an auto-resume by a commit. Also not
all conflicted/stopped state can be concluded with "commit" (think:
"rebase/am --skip").

I personally am leaning towards teaching "--continue" to "merge" and
"cherry-pick", while keeping only existing awareness of these two commands
in "commit" as historical mistakes (look for 'unlink(git_path("[A-Z_]*"))'
in builtin/commit.c). That would mean that in the long run, new users need
to learn only one thing: when "git Foo" stops because it needs help from
you resolving conflicts, after you help it by editing the files in your
working tree and making that with add/rm, you say "git Foo --continue" to
signal that you are done helping it.

And we do not have to worry about making "commit" only half-aware of
sequencer states.