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

[Elijah Newren <newren@gmail.com>]

On Mon, Mar 11, 2019 at 4:47 AM Duy Nguyen <pclouds@gmail.com> wrote:
>
> On Mon, Mar 11, 2019 at 6:16 PM Phillip Wood <phillip.wood123@gmail.com> wrote:
> >
> > Hi Duy
> >
> > 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').
>
> Interesting. I think this would be a good default if we have an escape
> hatch (which could even come later). I often wander off to some other
> branch and go back. But then half the time I end up forgetting I'm in
> a middle of something and just "git rebase --quit" :P
>
> Of course with git-stash (*) and git-worktree, I guess there's little
> reason to just switch away from a something-in-progress worktree. I'll
> try to implement this in the next round, unless someone objects.

No objection here; I like this idea.

> (*) I hope git-stash remembers and restores "something-in-progress"
> status. Dunno. Never used it much.
>
> > > +-C <new-branch>::
> > > +--force-create <new-branch>::
> > > +     Similar to `--create` except that if `<new-branch>` already
> > > +     exists, it will be reset to `<start-point>`. This is a
> > > +     convenient shortcut for:
> >
> > If we're renaming the options to be more meaningful then maybe we should
> > consider a different name for this one - it has nothing to do with
> > creating a branch. Maybe --reset or --update?
>
> -C can also create a new branch like -c though. --reset or --update
> does not convey that (and --update sounds a bit too safe). Another
> option is --recreate.

Maybe --recreate, but I don't see as much of a problem with the
original name you gave the option.  Which name is better probably
depends on how you envision its usage.  If you view this option as
only being used if/when '-c' fails (perhaps Phillip sees it that
way?), then it'd make sense to use --recreate instead.  But if you
think some might adopt a workflow where they just use -C without first
trying -c ("create this branch, and I don't care if I made it before
just create it here"), then --force-create makes sense.

Another option, possibly showing my lack of understanding why this
flag was useful in the first place: just drop this set of flags from
this command.  People can switch to the branch and then use reset
--hard <startpoint>, right?  Or (if they don't care about the reflog,
which they probably don't) delete the branch first and then recreate
it?  Not sure why we need to give another way to do these operations.
(In contrast, I see -c as being used frequently enough to have merit
even if it could be implemented as two separate commands.)

I don't have much of an opinion about which of these three is the best
option here (I'm slightly biased towards just jettisoning the option,
but I understand there might be a good reason for it even if I don't
know what it is), so really I'm just giving some food for thought on
this one.

> > > +-f::
> > > +--force::
> > > +     Proceed even if the index or the working tree differs from
> > > +     HEAD. Both the index and working tree are restored to match
> > > +     the switching target. This is used to throw away local
> > > +     changes.
> >
> > I'd always thought that --force meant "throw away my local changes if
> > they conflict with the new branch" not "throw them away regardless"
> > (which is better as it is deterministic). Maybe we can come up with a
> > clearer name here --discard-changes? At the moment --force does not
> > throw away conflicts properly (see the script below in my comments about
> > --merge).
>
> Yeah if you want to redefine --force, now is a very good time.
> Personally I'd rather have separate options than the "one catch all"
> --force (or worse, multiple of --force). I'll leave this for the
> community to decide.
>
> Adding Elijah too. He also had some concern about "git restore
> --force". Maybe he's interested in this as well.

I like Phillip's suggestion of --discard-changes.  I also like how he
came up with a simple testcase showing one bug each with checkout's
current -m and -f handling; we should fix those.