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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Elijah

On 11/03/2019 22:04, Elijah Newren wrote:
On Mon, Mar 11, 2019 at 1:51 PM Phillip Wood <phillip.wood123@xxxxxxxxx> wrote:
On 11/03/2019 17:24, Elijah Newren wrote:
On Mon, Mar 11, 2019 at 4:47 AM Duy Nguyen <pclouds@xxxxxxxxx> wrote:
On Mon, Mar 11, 2019 at 6:16 PM Phillip Wood <phillip.wood123@xxxxxxxxx> 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').

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.


Keeping this hunk since it's now relevant to the comment below...

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

You already suggested above (outside the context of --discard-changes)
that we should just error out if there is some special mid-operation
state (be it from a merge, cherry-pick, rebase, or bisect).  The user
can then manually resolve the operation first, or, perhaps use a
special override to force the switch command to proceed despite the
presence of mid-operation state.

Personally, I'm leaning towards --discard-changes operating within
that same context; I think that mid-operation special state should
require a more explicit and operation-specific step to remove (e.g.
rebase --quit).

I think that makes sense, I was wondering if --discard-changes should remove the state of a conflicted merge/single cherry-pick/revert but it is probably simpler and easier to understand just to error out. I think it should still clean up conflicts from other checkouts and applying stashes though as there is no other state in those cases.

Best Wishes

Phillip



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux