From: Phillip Wood Date: Mon, 11 Mar 2019 20:51:31 GMT Subject: Re: [PATCH v3 10/21] checkout: split part of it to new command 'switch' Message-ID: In-Reply-To: On 11/03/2019 17:24, Elijah Newren wrote: > On Mon, Mar 11, 2019 at 4:47 AM Duy Nguyen wrote: >> >> On Mon, Mar 11, 2019 at 6:16 PM Phillip Wood 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 :: >>>> +--force-create :: >>>> + Similar to `--create` except that if `` already >>>> + exists, it will be reset to ``. 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 , 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