On Tue, May 2, 2023 at 9:50 AM Junio C Hamano <gitster@xxxxxxxxx> wrote: > > Elijah Newren <newren@xxxxxxxxx> writes: > > > By the way, it was a problem that git-checkout wasn't updated to have > > the same safety that git-switch has. We should fix that. (It's on my > > todo list, along with adding other > > prevent-erroneous-command-while-in-middle-of-other-operation cases.) > > Yes. > > > I'm worried this is likely to lead us into confusing UI mismatches, > > and makes it harder to understand the appropriate rules of what can > > and cannot be done. A very simple "no switching branches in the > > middle of operations" is a very simple rule, and saves users from lots > > of headaches. > > > > Granted, expert users may understand that with the commit being the > > same, there is no issue. But expert users can use `git update-ref` to > > tweak HEAD, or edit .git/HEAD directly, and accept the consequences. > > Why do we need to confuse the UI for the sake of expert users who > > already have an escape hatch? > > > > More importantly, though... > > > >> Change the behavior of "git switch" and "git checkout" to no longer delete > >> merge metadata, nor prohibit the switch, if a merge is in progress and the > >> commit being switched to is the same commit the HEAD was previously set to. > > > > Even if there are conflicts? For rebases, cherry-picks, ams, and > > reverts too? (Does allowing this during rebases and whatnot mean that > > --abort becomes really funny? Does it mean that some commits are > > applied to one branch, and all commits are applied to another? What > > about autostashes? Does it interact weirdly with --update-refs? > > etc.) > > > > I think this change is premature unless it discusses all these cases, > > It is pretty much what I wanted to say about why we haven't done > this in <https://lore.kernel.org/git/xmqqpm7k6ojz.fsf@gitster.g/>, > so it makes two of us ;-). I didn't look at Tao's RFC patch but if > the way it determines "we are in a middle of conflicted merge and > we'll allow switching to the same commit only in this case" were > "the index has an unmerged entry", then it is an overly broad test > and the consequences of allowing the switch for these other merge-y > operations that are ongoing must be evaluated. He does tie it specifically to "is-this-a-merge-operation" (and actually doesn't check for conflicts at all since there are existing checks he leaves untouched). That certainly prevents some problems, but doesn't address my concerns. I think the usecase Tao presents has multiple simple workarounds, and I'm worried that the particular proposal might paint us into a corner. Personally, I think that before we consider a merge-specific-if-no-conflicts exception, someone should evaluate all the cases where exceptions could or should be allowed, get a documented story about them, and then if a consistent-ish UI is possible then propose patches to start taking us down this path.
