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

Clemens Buchacher [off-list ref] writes:

On Thu, Mar 15, 2012 at 10:36:22PM -0700, Junio C Hamano wrote:

quoted

+static const char message_advice_pull_before_push[] =
+	N_("Update was rejected because the tip of your current branch is behind\n"
+	   "the remote. Merge the remote changes (e.g. 'git pull') before\n"
+	   "pushing again. See the 'Note about fast-forwards' section of\n"
+	   "'git push --help' for details.");
+
+
+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.");

The first sentence of the above two warnings state the same thing, but
in different ways. Yet the difference does not reflect the different
situations. They should be the same, or maybe the first one should be
changed to the following variant of the second:

 "Updates were rejected because the tip of some of your branches are
 behind the remote branches with matching names."

That defeats the whole point of Christpher's patch and suggestion by Peff
in the original discussion.

They apply to two different situations. If your current branch is behind,
you get the first one, if your current branch is *NOT* behind, but some
others are, 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 thought they are clear
enough (that is why I sent the pach), but the wording may still need to be
polished, and updates are welcome.

We should not give advise_use_upstream if the user specified git push
--all. The advice_checkout_pull_push would make more sense in that case.

Yeah, "default_matching_used" variable should be looked at somewhere
around that, but I *think* the approach Christpher and Peff took (and I
agree with them) is to help solving the immediate problem the user has and
can address.  Deal with the current branch first (which would solve "the
current branch is behind" problem).  The next push may then show that
other branches are behind, and at that time the other advice will tell him
how to deal with it ("check out and fix them, and then push").

Again, updates are welcome.