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

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 16, 2012, 21:53 UTC
Message-ID: <7v1uosfc0p.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20120316214151.GA25092@ecki>

Clemens Buchacher <drizzd@aon.at> writes:

Show 20 quoted lines

> On Fri, Mar 16, 2012 at 05:03:58AM -0700, Junio C Hamano wrote:
>> Clemens Buchacher <drizzd@aon.at> writes:
>> ...
>> >> +static const char message_advice_use_upstream[] =
>> >> +	N_("Some of your local branches were stale with respect to their\n"
>> >> +	   "remote counterparts. If you did not intend to push these branches,\n"
>> >> +	   "you may want to set the 'push.default' configuration variable to\n"
>> >> +	   "'current' or 'upstream' to push only the current branch.");
>> >> +
>> >> +static const char message_advice_checkout_pull_push[] =
>> >> +	N_("Updates were rejected because the tip of some of your branches are\n"
>> >> +	   "behind the remote. Check out the branch and merge the remote\n"
>> >> +	   "changes (e.g. 'git pull') before pushing again. See the\n"
>> >> +	   "'Note about fast-forwards' section of 'git push --help'\n"
>> >> +	   "for details.");
>> >
> ...
> Sorry if I did not express myself well. I should have deleted the first
> message. I was not talking about the case where the current branch is
> rejected. I mean the two cases where other branches are rejected.

Oh, I see.  My reply ended up being very similar, though ;-)

These two apply to two different situations. In the code, you can see we switch between them like this:

Show 6 quoted lines

> +	case NON_FF_OTHER:
>  		if (default_matching_used)
>  			advise_use_upstream();
>  		else
>  			advise_checkout_pull_push();
> +		break;

This distinguishes the two cases _why_ you ended up pushing branches that are not your current branch. default_matching_used is set (eh, at least, "designed to be set"; the patch is based on my oooold patch whose details I do not recall offhand) only when the user said either "git push" or "git push $there" without explicit pathspec (i.e. "git push $there other" does not set it to true) and we end up using the "matching" semantics as it is the current built-in default.

If you tried to push other branch because you weren't aware of the matching default, you get the first advice, if you explicitly tried to push other branch, you get the second one. The suggested solutions are different.

Read each of them in isolation, imagining that you just saw your action resulted in a corresponding error condition. I think they are clear enough (that is why I sent the pach), but the wording may still need to be polished, and updates are welcome.