"Johannes Schindelin via GitGitGadget" <gitgitgad...@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.