Re: [PATCH v3 10/21] checkout: split part of it to new command 'switch'

[Elijah Newren <newren@gmail.com>]

On Tue, Mar 26, 2019 at 5:50 AM Duy Nguyen <pclouds@gmail.com> wrote:
> On Mon, Mar 11, 2019 at 6:16 PM Phillip Wood <phillip.wood123@gmail.com> wrote:
> > On 08/03/2019 09:57, Nguyễn Thái Ngọc Duy wrote:
> > > "git checkout" doing too many things is a source of confusion for many
> > > users (and it even bites old timers sometimes). To remedy that, the
> > > command will be split into two new ones: switch and
> > > something-to-checkout-paths.
> >
> > I think this is a good idea, thanks for working on it. I wonder if it
> > would be a good idea to have the new command refuse to checkout a new
> > branch if there is a cherry-pick/revert/merge/rebase in progress (with
> > an option to override the check) as switching branches in the middle of
> > one of those is likely to be confusing to users (if I do it it is
> > normally because I've forgotten that I've not run 'git whatever
> > --continue').
>
> Guys, I'm sorry for bringing this up again. Apparently I'm not quite
> done with 'git switch' yet (not sure if I will ever be).
>
> There is an interesting behavior in git-checkout (and of couse
> git-switch). When you do a successful switch, CHERRY_PICK_HEAD,
> REVERT_HEAD, MERGE_HEAD, MERGE_RR, MERGE_MSG, MERGE_MODE and
> SQUAHS_MSG, if exists, will be removed.
>
> This basically means that if you switch away, any cherry-pick, merge
> or revert in progress is destroyed (in the sense of "--quit" not
> "--abort" of course). All of this, I believe, involves merge conflicts
> so you can't easily switch away unless you allow to destroy unmerged
> entries. So it's still quite safe.
>
> However, it leaves me a funny feeling because some "work-in-progress"
> operations are destroyed, but some others (bisect, rebase) are not.
> This is git-checkout behavior and I will not change that. But do we
> want the same behavior in git-switch? Or do we want
> no-destroy-in-progress-whatsoever? Or
> destroy-all-commands-in-progress?

I thought we already decided that we'd abort the switch if there was
any operation-in-progress state?  Or are you asking what should we do
if the user explicitly overrides this error with e.g.
--ignore-in-progress?  In that case, I'd say that the reasonable thing
to do would be to leave all the state files alone.  If we make it
clear out the state, we're simply combining uncommon commands for the
user (<operation> --quit + git switch), which seems like a bad UI path
to go down.  Allowing them to switch to some other branch while
keeping all state files is something they can't do with any other
command, and while I hope people wouldn't want to do that much,
switching while keeping state files is something that can't be done
with combining other commands and thus at least makes sense as
something to consider providing.

> It may also be a good idea to attempt to describe the behavior we want
> in git-switch.txt. I think if the description gets too complicated,
> we're heading a wrong way. The current behavior so far could still be
> described as "work-in-progress operations related to merge conflicts
> are destroyed", or something along that line. But I'm not quite
> convinced it's easily understood.
>
> PS. git-reset shares the same behavior, but it's in a different boat,
> I think. Or maybe I should scrap/replace that one as well.

reset has traditionally been the home of
how-to-clear-in-progress-state.  e.g. aborting a merge or cherry-pick
or revert was 'reset --hard' (or later 'reset --merge'), skipping a
become-empty cherry-pick or rebase is still 'reset', etc.  So it's not
that surprising to me that it clears out state.  As to whether it
still should, we can address that along with...

> PPS. It may get trickier with cherry-pick which can pick a range of
> commits now, not just one. But I never used that feature much to know
> what I'm talking about.

Yeah, cherry-pick and revert and rebase all allow working on several
items.  It seems bad to me to abort/quit such operations without an
explicit '(cherry-pick|rebase|revert) --(abort|quit)'.  Back when
cherry-pick and revert only worked on a single commit, allowing 'reset
--hard' to clear state didn't seem that unreasonable even if it wasn't
the best UI.  So it all went there until people got used to 'rebase
--abort' and realized it was a better way and started spreading
--abort to other commands.  And you could kind of see us overlooking
that someone could cherry-pick --no-commit and then afterwards
checkout --force to put those changes somewhere else but I think it
was even dangerous/weird back then.  Once cherry-pick and revert
commands were extended to work with multiple commits like rebase does,
I just don't think it makes sense to allow reset to completely clear
out state files anymore.  Also, I don't want to check for one or
multiple commits and make them behave differently; I want git to
consistently require an explicit command-specific abort or quit (or
continue) to clear out the state associated with those
possibly-multi-step operations.  Maybe 'reset --hard' can continue
clearing out merge state just by force of habit, though I'd still
rather encourage 'merge --abort'.  But I don't think it makes sense to
let switch, reset, etc. remove multi-step progress state.