[PATCH 0/2] push: better error messages

DORMANTno replies

7 messages, 3 authors, 2016-06-15 · open the first message on its own page

[PATCH 0/2] push: better error messages

From: Matthieu Moy <hidden>
Date: 2016-06-15 22:50:41

Since it's likely to become the default in 1.8.0, I've played a bit
with "push.default = tracking". I was very happy to see that the flow

git init bla
git clone bla
git commit
git push

was working like a charm (it required a --all or so the first time),
but the "create a new branch and push it" is still a bit hard to get
for newbies.

[PATCH 1/2] should solve it, by providing a cut-and-paste ready
command to do the push.

While I was there, I also improved a bit other messages which gives
[PATCH 2/2]. I've kept the changes in separate patches, but they may
be squashed once reviewed.

Matthieu Moy (2):
  push: better error message when push.default = tracking
  push: better error messages for detached HEAD and "no destination"

 builtin/push.c |   27 ++++++++++++++++++++-------
 1 files changed, 20 insertions(+), 7 deletions(-)

-- 
1.7.4.1.142.g43604.dirty

[PATCH 1/2] push: better error message when push.default = tracking

From: Matthieu Moy <hidden>
Date: 2016-06-15 22:50:41

A common scenario is to create a new branch and push it (checkout -b &&
push [--set-upstream]). In this case, the user was getting "The current
branch %s has no upstream branch.", which doesn't help much.

Provide the user a command to push the current branch. To avoid the
situation in the future, suggest --set-upstream.

Signed-off-by: Matthieu Moy <redacted>
---
 builtin/push.c |   15 ++++++++++-----
 1 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/builtin/push.c b/builtin/push.c
index 31da418..210eb36 100644
--- a/builtin/push.c
+++ b/builtin/push.c

@@ -64,14 +64,19 @@ static void set_refspecs(const char **refs, int nr)
 	}
 }
 
-static void setup_push_upstream(void)
+static void setup_push_upstream(struct remote *remote)
 {
 	struct strbuf refspec = STRBUF_INIT;
 	struct branch *branch = branch_get(NULL);
 	if (!branch)
 		die("You are not currently on a branch.");
 	if (!branch->merge_nr || !branch->merge)
-		die("The current branch %s has no upstream branch.",
+		die("The current branch %s has no upstream branch.\n"
+		    "To push the current branch and set the remote as upstream, use\n"
+		    "\n"
+		    "    git push --set-upstream %s %s\n",
+		    branch->name,
+		    remote->name,
 		    branch->name);
 	if (branch->merge_nr != 1)
 		die("The current branch %s has multiple upstream branches, "

@@ -80,7 +85,7 @@ static void setup_push_upstream(void)
 	add_refspec(refspec.buf);
 }
 
-static void setup_default_push_refspecs(void)
+static void setup_default_push_refspecs(struct remote *remote)
 {
 	switch (push_default) {
 	default:

@@ -89,7 +94,7 @@ static void setup_default_push_refspecs(void)
 		break;
 
 	case PUSH_DEFAULT_UPSTREAM:
-		setup_push_upstream();
+		setup_push_upstream(remote);
 		break;
 
 	case PUSH_DEFAULT_CURRENT:

@@ -175,7 +180,7 @@ static int do_push(const char *repo, int flags)
 			refspec = remote->push_refspec;
 			refspec_nr = remote->push_refspec_nr;
 		} else if (!(flags & TRANSPORT_PUSH_MIRROR))
-			setup_default_push_refspecs();
+			setup_default_push_refspecs(remote);
 	}
 	errs = 0;
 	if (remote->pushurl_nr) {

-- 
1.7.4.1.142.g43604.dirty

[PATCH 2/2] push: better error messages for detached HEAD and "no destination"

From: Matthieu Moy <hidden>
Date: 2016-06-15 22:50:41

In both cases, give a hint on what to do next. We mention explicitely
"detached HEAD" since this is the keyword to look for in documentations.

Signed-off-by: Matthieu Moy <redacted>
---
 builtin/push.c |   12 ++++++++++--
 1 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/builtin/push.c b/builtin/push.c
index 210eb36..48fb050 100644
--- a/builtin/push.c
+++ b/builtin/push.c

@@ -69,7 +69,8 @@ static void setup_push_upstream(struct remote *remote)
 	struct strbuf refspec = STRBUF_INIT;
 	struct branch *branch = branch_get(NULL);
 	if (!branch)
-		die("You are not currently on a branch.");
+		die("You are not currently on a branch (detached HEAD).\n"
+		    "Please, checkout the branch you want to push first or specify it on the command line.");
 	if (!branch->merge_nr || !branch->merge)
 		die("The current branch %s has no upstream branch.\n"
 		    "To push the current branch and set the remote as upstream, use\n"

@@ -152,7 +153,14 @@ static int do_push(const char *repo, int flags)
 	if (!remote) {
 		if (repo)
 			die("bad repository '%s'", repo);
-		die("No destination configured to push to.");
+		die("No destination configured to push to.\n"
+		    "Either specify the URL from the command line or configure a remote repository using\n"
+		    "\n"
+		    "    git remote add <name> <url>\n"
+		    "\n"
+		    "and then push using the remote name like\n"
+		    "\n"
+		    "    git push <name>\n");
 	}
 
 	if (remote->mirror)

-- 
1.7.4.1.142.g43604.dirty

Re: [PATCH 2/2] push: better error messages for detached HEAD and "no destination"

From: Jay Soffian <hidden>
Date: 2016-06-15 22:50:41

On Mon, Feb 28, 2011 at 4:14 PM, Matthieu Moy [off-list ref] wrote:

quoted hunk

---
 builtin/push.c |   12 ++++++++++--
 1 files changed, 10 insertions(+), 2 deletions(-)

@@ -152,7 +153,14 @@ static int do_push(const char *repo, int flags)

       if (!remote) {
               if (repo)
                       die("bad repository '%s'", repo);
-               die("No destination configured to push to.");
+               die("No destination configured to push to.\n"

The dangling preposition is awkward. How about:

  No configured push destination.

Or

  Push destination not configured.

+                   "Either specify the URL from the command line or configure a remote repository using\n"

Maybe "Either specify the URL explicitly or configure..."

+                   "\n"
+                   "    git remote add <name> <url>\n"
+                   "\n"
+                   "and then push using the remote name like\n"

s/like//

+                   "\n"
+                   "    git push <name>\n");

Well, I just bikeshedded that 3 different colors, didn't I? You're welcome. :-)

j.

Re: [PATCH 2/2] push: better error messages for detached HEAD and "no destination"

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:50:41

Matthieu Moy [off-list ref] writes:

quoted hunk

In both cases, give a hint on what to do next. We mention explicitely
"detached HEAD" since this is the keyword to look for in documentations.

Signed-off-by: Matthieu Moy <redacted>
---
 builtin/push.c |   12 ++++++++++--
 1 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/builtin/push.c b/builtin/push.c
index 210eb36..48fb050 100644
--- a/builtin/push.c
+++ b/builtin/push.c

@@ -69,7 +69,8 @@ static void setup_push_upstream(struct remote *remote)
 	struct strbuf refspec = STRBUF_INIT;
 	struct branch *branch = branch_get(NULL);
 	if (!branch)
-		die("You are not currently on a branch.");
+		die("You are not currently on a branch (detached HEAD).\n"
+		    "Please, checkout the branch you want to push first or specify it on the command line.");

I agree that it is a good thing to suggest specifying what to push on the
command line, but I don't think this "checkout first" is particularly a
good advice, as it requires two-step operation that is quite a roundabout
way of doing what the user wanted to do: a very specific checkout followed
by re-running of a nonspecific push.

quoted hunk

@@ -152,7 +153,14 @@ static int do_push(const char *repo, int flags)
 	if (!remote) {
 		if (repo)
 			die("bad repository '%s'", repo);
-		die("No destination configured to push to.");
+		die("No destination configured to push to.\n"
+		    "Either specify the URL from the command line or configure a remote repository using\n"
+		    "\n"
+		    "    git remote add <name> <url>\n"
+		    "\n"
+		    "and then push using the remote name like\n"
+		    "\n"
+		    "    git push <name>\n");
 	}

This one would be helpful.

[PATCH 1/2 v2] push: better error messages when push.default = tracking

From: Matthieu Moy <hidden>
Date: 2016-06-15 22:50:41

A common scenario is to create a new branch and push it (checkout -b &&
push [--set-upstream]). In this case, the user was getting "The current
branch %s has no upstream branch.", which doesn't help much.

Provide the user a command to push the current branch. To avoid the
situation in the future, suggest --set-upstream.

While we're there, also improve the error message in the "detached HEAD"
case. We mention explicitly "detached HEAD" since this is the keyword to
look for in documentations.

Signed-off-by: Matthieu Moy <redacted>
---
Compared to v1, the detached HEAD case joined this patch since it
became semantically linked (requires the "remote" extra parameter),
and doesn't mention the possibility to checkout another branch before
pushing it (cf. Junio).

 builtin/push.c |   21 +++++++++++++++------
 1 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/builtin/push.c b/builtin/push.c
index 31da418..c949187 100644
--- a/builtin/push.c
+++ b/builtin/push.c

@@ -64,14 +64,23 @@ static void set_refspecs(const char **refs, int nr)
 	}
 }
 
-static void setup_push_upstream(void)
+static void setup_push_upstream(struct remote *remote)
 {
 	struct strbuf refspec = STRBUF_INIT;
 	struct branch *branch = branch_get(NULL);
 	if (!branch)
-		die("You are not currently on a branch.");
+		die("You are not currently on a branch (detached HEAD).\n"
+		    "To push a specific branch and set the remote as upstream, use\n"
+		    "\n"
+		    "    git push --set-upstream %s <branch-name>\n",
+		    remote->name);
 	if (!branch->merge_nr || !branch->merge)
-		die("The current branch %s has no upstream branch.",
+		die("The current branch %s has no upstream branch.\n"
+		    "To push the current branch and set the remote as upstream, use\n"
+		    "\n"
+		    "    git push --set-upstream %s %s\n",
+		    branch->name,
+		    remote->name,
 		    branch->name);
 	if (branch->merge_nr != 1)
 		die("The current branch %s has multiple upstream branches, "

@@ -80,7 +89,7 @@ static void setup_push_upstream(void)
 	add_refspec(refspec.buf);
 }
 
-static void setup_default_push_refspecs(void)
+static void setup_default_push_refspecs(struct remote *remote)
 {
 	switch (push_default) {
 	default:

@@ -89,7 +98,7 @@ static void setup_default_push_refspecs(void)
 		break;
 
 	case PUSH_DEFAULT_UPSTREAM:
-		setup_push_upstream();
+		setup_push_upstream(remote);
 		break;
 
 	case PUSH_DEFAULT_CURRENT:

@@ -175,7 +184,7 @@ static int do_push(const char *repo, int flags)
 			refspec = remote->push_refspec;
 			refspec_nr = remote->push_refspec_nr;
 		} else if (!(flags & TRANSPORT_PUSH_MIRROR))
-			setup_default_push_refspecs();
+			setup_default_push_refspecs(remote);
 	}
 	errs = 0;
 	if (remote->pushurl_nr) {

-- 
1.7.4.1.142.ge03e1

[PATCH 2/2 v2] push: better error message when no remote configured

From: Matthieu Moy <hidden>
Date: 2016-06-15 22:50:41

Signed-off-by: Matthieu Moy <redacted>
---
"No configured push destination." (Jay Soffian) and reword.

 builtin/push.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/builtin/push.c b/builtin/push.c
index c949187..b4fba7a 100644
--- a/builtin/push.c
+++ b/builtin/push.c

@@ -156,7 +156,14 @@ static int do_push(const char *repo, int flags)
 	if (!remote) {
 		if (repo)
 			die("bad repository '%s'", repo);
-		die("No destination configured to push to.");
+		die("No configured push destination.\n"
+		    "Either specify the URL from the command-line or configure a remote repository using\n"
+		    "\n"
+		    "    git remote add <name> <url>\n"
+		    "\n"
+		    "and then push using the remote name\n"
+		    "\n"
+		    "    git push <name>\n");
 	}
 
 	if (remote->mirror)

-- 
1.7.4.1.142.ge03e1

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help