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

On 03/16/2012 06:36 AM, Junio C Hamano wrote:

+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.");

Hi,

Clemens' observation that there are unnecessary differences between
"message_advice_use_upstream" and "message_advice_checkout_pull_push"
is valid. There also was a grammatical error in message_advice_checkout_pull_push
("the tip ... are behind") and some tense/number inconsistencies.

I think the following can be squashed into 'fixup push-non-ff advice':

- always start with "Updates were rejected", i.e. explain what is why
  git is talking
- consistently use present tense to talk about stuff which is still true
- mention that branches to be pushed can be specified (add
  " explicitly specify branches to push or" in
  "you may want to set the 'push.default' configuration variable")
- use the simpler "tip of your branch is behind the remote" instead of the more 
  complicated and longer "some of your branches are stale with respect to their 
  remote counterparts".
- resolve the "tip ... are" problem by using singular and talking about
  a single branch. This way there is no conflict with the following 
  sentence which talks about checking out a single branch.
- rewrap the text to 72 lines (standard TeX paragraph width).
  (One line is 73 characters, but it seems better than the 
  alternative which makes the text take an extra line).

[I know that this mixes whitespace/layout changes with the rest, but the texts were mostly rewritten anyway.]

Zbyszek

------ 8< --------
From ef8d15494d518df809e4a822af0d0e1c4008c91e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <redacted>
Date: Sat, 17 Mar 2012 18:00:42 +0100
Subject: [PATCH] fixup! fixup push-non-ff advice

---
 builtin/push.c |   25 +++++++++++--------------
 1 file changed, 11 insertions(+), 14 deletions(-)

diff --git a/builtin/push.c b/builtin/push.c
index 511a3ba..4c5b52b 100644
--- a/builtin/push.c
+++ b/builtin/push.c

@@ -142,24 +142,21 @@ static void setup_default_push_refspecs(struct remote *remote)
 }
 
 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.");
-
+	N_("Update was rejected because the tip of your current branch is behind the\n"
+	   "remote. Merge the remote changes (e.g. 'git pull') before pushing again.\n"
+	   "See the 'Note about fast-forwards' in '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.");
+	N_("Updates were rejected because a tip of your branch is behind the remote.\n"
+	   "If you did not intend to push that branch, you may want to explicitly\n"
+	   "specify branches to push or set the 'push.default' configuration variable"
+	   "to 'current' or 'upstream' to always 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.");
+	N_("Updates were rejected because a tip of your branch is behind the remote.\n"
+	   "Check out this branch and 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 void advise_pull_before_push(void)
 {

-- 
1.7.10.rc0.162.g5dce3

------ >8 --------