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

[Phillip Wood <phillip.wood123@gmail.com>]

On 11/03/2019 17:24, Elijah Newren wrote:
> 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?), 

Yes I was thinking it was used to reset an existing branch in order to 
reuse the name, --recreate is a good suggestion in that context.

> 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 tend to agree with this but that's probably because I don't really use 
checkout -B. I'm not sure if it's widely used or not. I do find checkout 
-b convenient though.

> 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.

With regard to discarding conflicts, do we want it to clear up any state 
associated with the conflicts (like reset)? They rarely happen in 
isolation, there's a MERGE_HEAD or CHERRY_PICK_HEAD etc. I'm not sure 
what it should do in the middle of a rebase or when cherry-picking a 
range of commits. I think it would be surprising if it was the 
equivalent of rebase/cherry-pick --quit but just clearing the conflicts 
in those contexts may not be very useful in practice.

Best Wishes

Phillip