Re: [PATCH 0/3] commit: fix advice for empty commits during rebases

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

"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:

> In dcb500dc16c (cherry-pick/revert: advise using --skip, 2019-07-02), we
> introduced a helpful message that suggests to run git cherry-pick --skip 
> (instead of the previous message that talked about git reset) when a
> cherry-pick failed due to an empty patch.
>
> However, the same message is displayed during a rebase, when the patch
> to-be-committed is empty. In this case, git reset would also have worked,
> but git cherry-pick --skip does not work. This is a regression introduced in
> this cycle.
>
> Let's be more careful here.
>
> Johannes Schindelin (3):
>   cherry-pick: add test for `--skip` advice in `git commit`
>   sequencer: export the function to get the path of `.git/rebase-merge/`
>   commit: give correct advice for empty commit during a rebase

Overall they looked nicely done.  The last one may have started its
life as "let's fix advice for empty", but no longer is.

The old code used the sequencer_in_use boolean to tell between two
states ("are we doing a single pick, or a range pick?"), but now we
have another boolean rebase_in_progress, and depending on the shape
of the if statements existing codepaths happen to have, these two
are combined in different ways to deal with new states.  E.g. some
places say

	if (rebase_in_progress && !sequencer_in_use)
		we are doing a rebase;
	else
		we are doing a cherry-pick;

and some others say

	if (sequencer_in_use)
		we are doing a multi pick;
	else if (rebase_in_progress)
		we are doing a rebase;
	else
		we are doing a single pick;

I wonder if it makes the resulting logic simpler to reason about if
these combination of two variables are rewritten to use a single
variable that enumerates three (or is it four, counting "we are
doing neither a cherry-pick or a rebase"?) possible state.

Other than that, looked sensible.  Will queue.

Thanks.