Felipe Contreras <felipe.contreras@xxxxxxxxx> writes: >> 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. > > I don't agree with the above. > > The error I propose is just: > > The pull was not fast-forward, please either merge or rebase. > > That's it. Nothing more. It says "why we are stopping." quite well. It would be a good message to use as the first part of the three-part message I mentioned above. > I explained that was the final end goal in my list of steps [1]. I do > not think any suggestion for commands or configurations belongs in a > *permanent* error message. In the design I have in mind in the message you are responding to, the users who haven't told their choice to Git would be the only folks who get all three. You want to let the user express: "I do not want to choose either rebase or merge. I want 'pull' to fail when it needs to deal with non-ff history. But I do not need to be told about command line option and configuration every time." I said I don't (I view that disabling half the "git pull" just a safe fallback behaviour until the user chooses between merge and rebase), but if we wanted to offer it as a valid choice to users, we can do so. We just make it possible to squelch the latter two parts of the three-part message---you leave pull.rebase unconfigured and squelch the latter two parts of the message, and you got the "stop me, I do not merge or rebase, but don't even tell me how to further configure" already. I agree the latter two should not be part of *permanent* error message. And my suggestion did not intend to make them so---it should have been quite obvious to who read the message you are responding to through to the end and understood what it said. Now, how would we make it possible to squelch the latter two parts? It is not a good idea to introduce pull.mode=ff-only for that purpose (pull.mode=rebase and pull.rebase=yes would unnecessarily become two redundant ways to do the same thing if we added a new pull.mode variable). But there is an established way used in this project when we allow squelching overly-helpful help messages, and we can apply it here as well. That way: - unconfigured folks would get all the three parts of the messages, just like the current system. - if you tell rebase or merge, you do not see any. - if you do not choose between rebase or merge, you can still squelch the latter two by setting advice.pullNonFF to false. The last one is "keep the more dangerous half of 'git pull' disabled, without getting told how to enable it over and over", which is what you want to be able to specify. > The reason "pull.mode=ff-only" needs to be introduced is that > --ff-only doesn't work. Otherwise there's no way the user cannot > select the "safe default" mode. It has absolutely nothing to do with > what we present the user with. I too initially thought that pull.mode may be needed, but probably I was wrong. I do think this can be done without pull.mode at all, at least in two ways, without adding different ways to do the same thing. - When pull.rebase is set to 'no' and pull.ff is set to 'only', "git pull" that sees a non-ff history should error out safely. The user is telling that their preference is to merge, but the difference between merge and rebase does not really matter because pull.ff=only would mean we forbid merges of non-ff history anyway. The message you'd get would be "fatal: Not possible to fast-forward, aborting." though. - Or with the advice that hides the latter two points, a user can unset pull.rebase and set the advice.pullNonFF to false to get the same behaviour (i.e. disable the more dangerous half of "pull") with just the "we stopped" error message. I think either of these are close enough to what you want, and I think the latter gives us more flexibility in how we tone down the message with advice.pullNonFF.
