Re: [PATCH v2 02/14] pull: improve default warning

Elijah Newren <newren@gmail.com> writes:

I tend to agree that the endgame state I want is pretty similar to what Felipe wants. The seeming confusion is probably due to what exactly I mean by "default" is different from what he means.

I view the proposed "for unconfigured users, pull dies, and tells them to choose between rebase or merge before it can continue, when faced with a non-ff history" as a safe fallback behaviour until the users make their choice.

It is a safe fallback to disable the more dangerous half of the command until the user gives enough information to the command to do its job without damaging the resulting history; it is not something the users would actively want to choose.

And that is what I meant by the default behaviour.

And when we stop in such a manner, it is sensible to give an error message telling them

 - why we are stopping,

 - what they can do to move the immediate situation forward
   (i.e. command line option that lets them choose), and

 - what they can do to make their choice permanent so that they
   would never see the command stop when facing a non-ff history
   (i.e. the configuration variables).

Up to this point, I think both of us agree with the above.

We start to differ after this point. I would want to see only "rebase" and "merge in the "choice" in the above list. Felipe, if I understand correctly, wants to add the third one, "ff-only", which means the more dangerous half of "pull" is disabled by default.

I do not want to include that choice, as it would mean the more familiar pull.rebase=yes/no would no longer work, and we'd need to introduce a new variable, like pull.mode=ff-only. It would mean for those who use "pull" to consolidate histories on their side and other people's side, whether they use rebase or merge, would now have two ways to do the same thing (i.e. pull.rebase=no or pull.mode=merge). That is just making the end user experience more complex than necessary.

Without introducing pull.mode, the only thing the users cannot do, as far as I can tell, is to explicitly ask for the behaviour of an unconfigured user (i.e. error out when faced with non-ff history) without being told about the way to use configuration variable to permanently record their choice. Other than that, the existing pull.rebase=yes/no is perfectly adequate. Felipe seems to think otherwise and wants not just the "safe fallback behaviour", but can explicitly be configured using pull.mode=ff-only (and if that is not what Felipe thinks, perhaps we are already in agreement without realizing it).

Thinking about it again, I guess pull.rebase=yes/no plus a new advise entry can easily give what Felipe seems to want. We teach "git pull", when it is not told to rebase or merge (either via the command line --[no-]rebase or via pull.rebase configuration) and when it sees a non-ff history, to

 - error out with a message telling the user that we are stopping,
   because the two histories needs consolidating to conclude this
   pull, but the two available ways would give vastly different
   result.

 - check the advice.pullNonFF configuration (as usual, it defaults
   to true) and if true, further tell the user that

    (1) they can either run "git pull --rebase" or "git pull
        --no-rebase",

    (2) they can configure their choice permanently with
        pull.rebase=yes/no, and

    (3) they can leave "pull" in the half-disabled state but stop
        seeing this message by setting advice.pullNonFF to false.

The last part (i.e. advice.pullnonFF) is the only new thing I said in the various discussions on this topic (simply because I just came up with the idea). Everything else I've already said it elsewhere.

I dunno.