Git Mailing List Archive mirror
 help / color / mirror / Atom feed
From: Tao Klerks <tao@klerks.biz>
To: Felipe Contreras <felipe.contreras@gmail.com>
Cc: Elijah Newren <newren@gmail.com>,
	Tao Klerks via GitGitGadget <gitgitgadget@gmail.com>,
	git@vger.kernel.org
Subject: Re: [PATCH] RFC: switch: allow same-commit switch during merge if conflicts resolved
Date: Mon, 8 May 2023 10:30:51 +0200	[thread overview]
Message-ID: <CAPMMpojTjFn7JCo8QsDcOJf6NoJYASbV1bL_JxDhUr7DS12DJg@mail.gmail.com> (raw)
In-Reply-To: <64581fc358ede_4e6129442@chronos.notmuch>

On Mon, May 8, 2023 at 12:01 AM Felipe Contreras
<felipe.contreras@gmail.com> wrote:
>
> Elijah Newren wrote:
> > On Wed, May 3, 2023 at 10:01 PM Tao Klerks <tao@klerks.biz> wrote:
>
> > > If we are comfortable changing the behavior of branch checkout to be
> > > safe-and-limiting like switch, then that should be almost as simple as
> > > removing that condition.
> >
> > I've never heard a dissenting vote against this
>
> Here is my dissenting vote: I'm against this change.
>
> If I want to use a high-level command meant for novices, I use `git switch`. If
> instead I simply want to switch to a different commit and I want git to shut up
> about it, then I use `git checkout`.

Thank you for your perspective on the relationship between these commands.

I don't fully share this perspective, in two ways:
- In my experience most novices don't see or know about "git switch"
at all - the vast majority of the internet is still stuck on "git
checkout", as are existing users. Google search result counts are of
course a poor metric of anything, but compare 100k for "git switch" to
2.4M for "git checkout".
- As far as I can tell, "git switch" and "git restore" have exactly
the same power and expressiveness (except specifically the lack of
"git switch --force" support for bulldozing ongoing merges) - they are
just as much "expert" tools as "git checkout"; the main way they
differ is that they are clearer about what they're doing / what
they're for. I'd love to see "git checkout" deprecated one day,
although I'm not sure I'll live to see it happen :)

>
> You want to strip away the options for experts, in search for what?

What *I* want is a generally safe system, in which you don't have to
be an expert to avoid causing yourself problems, especially losing
work.

The specific example that motivated my wanting to change "git
checkout" here was the case of a normal (non-expert, non-novice) user
who is used to doing "git checkout -b
actually-those-changes-I-made-shouldnt-go-on-the-branch-I-was-working-on-yet".
In their day-to-day work, that action will always have achieved
exactly what they wanted. The day they make exactly the same
invocation just before they commit a merge, it will do something
completely different and confusing - it will destroy a merge state,
and result in the commit, shortly after, being a regular non-merge
commot. Leveraging that committed tree in a merge commit *is* indeed
an expert action, and most novice and maybe intermediate users will
instead find themselves cursing git, and starting the merge again from
scratch - if they even notice the problem. If they don't notice the
problem, then they will instead have a new and fascinating source of
merge conflicts at some future time.

In general I *will* be willing to make things a little harder for
experts in favor of novices - absolutely.

That said, I don't believe the (new) change proposed here strips away
*useful* options from experts at all. When I said "safe-and-limiting",
I meant it in the most literal way - that there are some operations
that could be performed before and would achieve certain outcomes that
won't be possible afterwards. What I didn't mean to imply is that
those options are *valuable* to anyone - even to experts.

>
> If there was a way of doing:
>
>   git -c core.iknowwhatimdoing=true checkout $whatever
>
> Then I wouldn't oppose such change.

I know I keep wavering back and forth on this, my apologies for my
inconstancy: *I once again think adding support for "--force" (to
checkout and switch) with ongoing operations makes sense.*

This does not achieve exactly what you seem to be suggesting above,
for two reasons:
1. It could not be implicit in config, but rather would need to be
explicit in the command
2. The outcome of using --force is not exactly the same as "git
checkout" without it (but that's a good thing)

I would (and will) argue that not achieving exactly what you propose
*is OK* because the behavior of "git checkout", without "--force",
when there is a (merge, rebase, cherry-pick, am, bisect) operation in
course, especially the way that behavior differs from when "--force"
is specified, is *not useful* - even to expert users.

I will provide a table of behaviors with a proposed patch in a few
days, but basically the main behavior we're taking away is a
one-command behavior of "switch branch and remove the (merge,
cherry-pick) in-progress state". The explicit equivalent is and will
continue to be "git [merge|cherry-pick] --quit && git checkout" -
leaving the in-progress merge changes in the index, but switching to
the specified branch.

My expectation is that this is not something even expert users find
themselves doing... ever. But I would like to know about it if I'm
wrong of course!

Something that I *do* see quite a lot in the test suite, and is
prompting my turn-about on "--force" support, is "git checkout -f
whatever" as a shorthand for "just get my worktree to the state of
that branch, regardless of the current ongoing operation".

This shorthand happens to *fail to work correctly* during a rebase
(clearing of the rebasing state was never implemented), but I believe
that has more to do with priorities and scope of changes than
intentional "let's set an extra-confusing trap for rebase" reasons.
The resulting state, where you have switched to the requested branch
and discarded any local changes, but are still in a rebase,
potentially with pending rebase sequence steps to complete, is not one
that I can see even expert users making constructive use of.

Generally, the "allow 'checkout --force' to destroy in-progress
operation states" behavior looks like an expert shortcut worth
preserving, and improving/fixing in the case of rebase.

>
> But this is not the proposal. The proposal is to break backwards compatibility
> for expert users with no way to retain the existing behavior.
>

It is true that the (updated) proposal closes the doors on *specific*
behaviors as a single command, requiring them to instead be spread
across two commands. However, I believe that those are effectively
*unused behaviors*, and that the increase in consistency and safety,
for all users, by far outweighs the cost of this particular break in
backwards compatibility.

I am, again, very interested in anything I might be missing!

> Generally, breaking backwards compatibility for no reason is frowned upon.
>

I absolutely understand and agree that breaking backwards
compatibility *for no reason* is never the right thing - and I take
note that being clear about exactly what the reasons are, and what the
costs are, *before* talking about doing it and asking for opinions, is
advisable and something that I failed to do sensibly here.

Thanks again for the feedback, please let me know if you know of any
useful expert use cases that I *am* missing in this updated proposal.

  reply	other threads:[~2023-05-08  8:31 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-02  6:27 [PATCH] RFC: switch: allow same-commit switch during merge if conflicts resolved Tao Klerks via GitGitGadget
2023-05-02 15:55 ` Elijah Newren
2023-05-02 16:50   ` Junio C Hamano
2023-05-03  0:34     ` Elijah Newren
2023-05-04  5:01   ` Tao Klerks
2023-05-05  5:06     ` Tao Klerks
2023-05-07  2:57       ` Elijah Newren
2023-05-07  2:48     ` Elijah Newren
2023-05-07 22:01       ` Felipe Contreras
2023-05-08  8:30         ` Tao Klerks [this message]
2023-05-08 16:13           ` Felipe Contreras
2023-05-08 16:58             ` Tao Klerks
2023-05-08 19:18               ` Junio C Hamano
2023-05-09  1:55               ` Felipe Contreras
2023-05-08 10:44       ` Tao Klerks
2023-05-11  7:06         ` Elijah Newren
2023-05-21 20:08         ` Tao Klerks

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAPMMpojTjFn7JCo8QsDcOJf6NoJYASbV1bL_JxDhUr7DS12DJg@mail.gmail.com \
    --to=tao@klerks.biz \
    --cc=felipe.contreras@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=newren@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).