Re: [PATCH v2] push: Provide situational hints for non-fast-forward errors

[Junio C Hamano <gitster@pobox.com>]

Christopher Tiwald <christiwald@gmail.com> writes:

> Pushing a non-fast-forward update to a remote repository will result in
> an error, but the hint text doesn't provide the correct resolution in
> every case. Give better resolution advice in three push scenarios:
>
> 1) If you push a non-fast-forward update to your current branch, you
> should merge remote changes with 'git pull' before pushing again.

I have always found "update *to* your current branch" very strange
phrasing (the earlier one said "to HEAD", but it amounts to the same
thing).  You do not push *to* your branch.  You push your branch to
somewhere else (namely, remote).  I would understand if it said "If your
push of your current branch triggers a non-ff error, ...", though.

> 	  non-ff errors on other branches, but not current

I think the change in this patch comes from a realization that a blanket
"Here is all you need to know for any and all non-ff error cases" is not
very useful, and it feels like it is going backwards to squash the "non-ff
but not the current" into one category.

The user may have been using the matching default, gets the use-upstream
advice and realizes that she is trying to push branches other than what
she wanted to push, and may say "git push $there master" to push only that
branch out.  Then she thinks she learned enough to squelch the message in
$HOME/.gitconfig.

She may have another project with remote.$there.push set to push more than
one branches out (say, master and next), and while on 'master', may hit
another "non-ff on other" instance, because her 'next' was stale.

She never gets a chance to see the other checkout-pull-push message, does
she?

>  There is one aspect about this patch about which I'm unsure: What to
>  do with users who've set "advice.pushNonFastForward = false" already.

The change in this patch is merely clarifying what pushNonFastForward
advise has already taught them ("Non-ff was rejected; the manual will tell
you what you wanted to do") by dividing them into three categories and
giving different advices to these categories.  As the user says he
understood what he is doing, I think squelching all of them is a sane
choice.

> +static const char message_advice_pull_before_push[] =
> +	N_("Updates were rejected because the tip of your current branch is behind\n"
> +	   "its remote counterpart. Merge the remote changes (e.g. 'git pull')\n"
> +	   "before pushing again.\n"
> +	   "See the 'Note about fast-forwards' in 'git push --help' for details.");
> +
> +static const char message_advice_use_upstream[] =
> +	N_("Updates were rejected because a pushed branch tip is behind its remote\n"
> +	   "counterpart. If you did not intend to push that branch, you may want to\n"
> +	   "specify branches to push or set the 'push.default' configuration\n"
> +	   "variable to 'current' or 'upstream' to push only the current branch.");
> +
> +static const char message_advice_checkout_pull_push[] =
> +	N_("Updates were rejected because a pushed branch tip is behind its remote\n"
> +	   "counterpart. Check out this branch and merge the remote changes\n"
> +	   "(e.g. 'git pull') before pushing again.\n"
> +	   "See the 'Note about fast-forwards' in 'git push --help' for details.");

In any case, the updated messages read much better than the current non-ff
one (or the previous round for that matter).