threads / patch / 23184

patchpull: mention "pull", not "fetch" in the error message.

Subject: [PATCH] pull: mention "pull", not "fetch" in the error message.

## tl;dr

7 messages between Mar 26, 2010 and Mar 28, 2010. Diffs are folded; open one to read it.

replies: 6people: 5as markdown or json

Matthieu Moy· Mar 26, 2010, 13:38 UTC · lore

For newbies who've just been taught "git push", the error message "Where do you want to fetch from today?" is indeed confusing. Change it to "Where do you want to pull from today?" in case fetch was called from pull.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
 builtin/fetch.c |    6 +++++-
 git-pull.sh     |    3 ++-
 2 files changed, 7 insertions(+), 2 deletions(-)

Show changes to 2 files +7 −2

builtin/fetch.c, git-pull.sh

diff --git a/builtin/fetch.c b/builtin/fetch.c
index 957be9f..f3246f5 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -842,8 +842,12 @@ static int fetch_one(struct remote *remote, int argc, const char **argv)
 	int ref_nr = 0;
 	int exit_code;
 
+	char *cmd = getenv("GIT_USER_COMMAND");
+	if (cmd == NULL || cmd[0] == '\0')
+		cmd = "fetch";
+
 	if (!remote)
-		die("Where do you want to fetch from today?");
+		die("Where do you want to %s from today?", cmd);
 
 	transport = transport_get(remote, NULL);
 	transport_set_verbosity(transport, verbosity, progress);
diff --git a/git-pull.sh b/git-pull.sh
index 1a4729f..abc233b 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -216,7 +216,8 @@ test true = "$rebase" && {
 	done
 }
 orig_head=$(git rev-parse -q --verify HEAD)
-git fetch $verbosity $progress --update-head-ok "$@" || exit 1
+GIT_USER_COMMAND=pull \
+    git fetch $verbosity $progress --update-head-ok "$@" || exit 1
 
 curr_head=$(git rev-parse -q --verify HEAD)
 if test -n "$orig_head" && test "$curr_head" != "$orig_head"

-- 
1.7.0.2.204.g8940d.dirty

Michael J Gruber· Mar 26, 2010, 14:18 UTC · re: Matthieu Moy · lore

Re: [PATCH] pull: mention "pull", not "fetch" in the error message.

Matthieu Moy venit, vidit, dixit 26.03.2010 14:38:

> For newbies who've just been taught "git push", the error message

Even though newbies don't read git.git's commit messages, you may want to say "pull" here ;)

One way or the other, nothing beats the error messages in ident.c ;)

Show 42 quoted lines

> "Where do you want to fetch from today?" is indeed confusing. Change it
> to "Where do you want to pull from today?" in case fetch was called from
> pull.
> 
> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
> ---
>  builtin/fetch.c |    6 +++++-
>  git-pull.sh     |    3 ++-
>  2 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/builtin/fetch.c b/builtin/fetch.c
> index 957be9f..f3246f5 100644
> --- a/builtin/fetch.c
> +++ b/builtin/fetch.c
> @@ -842,8 +842,12 @@ static int fetch_one(struct remote *remote, int argc, const char **argv)
>  	int ref_nr = 0;
>  	int exit_code;
>  
> +	char *cmd = getenv("GIT_USER_COMMAND");
> +	if (cmd == NULL || cmd[0] == '\0')
> +		cmd = "fetch";
> +
>  	if (!remote)
> -		die("Where do you want to fetch from today?");
> +		die("Where do you want to %s from today?", cmd);
>  
>  	transport = transport_get(remote, NULL);
>  	transport_set_verbosity(transport, verbosity, progress);
> diff --git a/git-pull.sh b/git-pull.sh
> index 1a4729f..abc233b 100755
> --- a/git-pull.sh
> +++ b/git-pull.sh
> @@ -216,7 +216,8 @@ test true = "$rebase" && {
>  	done
>  }
>  orig_head=$(git rev-parse -q --verify HEAD)
> -git fetch $verbosity $progress --update-head-ok "$@" || exit 1
> +GIT_USER_COMMAND=pull \
> +    git fetch $verbosity $progress --update-head-ok "$@" || exit 1
>  
>  curr_head=$(git rev-parse -q --verify HEAD)
>  if test -n "$orig_head" && test "$curr_head" != "$orig_head"

Matthieu Moy· Mar 26, 2010, 15:02 UTC · re: Michael J Gruber · lore

Re: [PATCH] pull: mention "pull", not "fetch" in the error message.

Michael J Gruber <git@drmicha.warpmail.net> writes:

Show 5 quoted lines

> Matthieu Moy venit, vidit, dixit 26.03.2010 14:38:
>> For newbies who've just been taught "git push", the error message
>
> Even though newbies don't read git.git's commit messages, you may want
> to say "pull" here ;)

Right. I was just two letters away from the right message ;-).

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Matthieu Moy· Mar 26, 2010, 15:03 UTC · re: Michael J Gruber · lore

[PATCH v2] pull: mention "pull", not "fetch" in the error message.

For newbies who've just been taught "git pull", the error message "Where do you want to fetch from today?" is indeed confusing. Change it to "Where do you want to pull from today?" in case fetch was called from pull.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
Just fixed a typo in commit message since v1.

 builtin/fetch.c |    6 +++++-
 git-pull.sh     |    3 ++-
 2 files changed, 7 insertions(+), 2 deletions(-)

Show changes to 2 files +7 −2

builtin/fetch.c, git-pull.sh

diff --git a/builtin/fetch.c b/builtin/fetch.c
index 957be9f..f3246f5 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -842,8 +842,12 @@ static int fetch_one(struct remote *remote, int argc, const char **argv)
 	int ref_nr = 0;
 	int exit_code;
 
+	char *cmd = getenv("GIT_USER_COMMAND");
+	if (cmd == NULL || cmd[0] == '\0')
+		cmd = "fetch";
+
 	if (!remote)
-		die("Where do you want to fetch from today?");
+		die("Where do you want to %s from today?", cmd);
 
 	transport = transport_get(remote, NULL);
 	transport_set_verbosity(transport, verbosity, progress);
diff --git a/git-pull.sh b/git-pull.sh
index 1a4729f..abc233b 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -216,7 +216,8 @@ test true = "$rebase" && {
 	done
 }
 orig_head=$(git rev-parse -q --verify HEAD)
-git fetch $verbosity $progress --update-head-ok "$@" || exit 1
+GIT_USER_COMMAND=pull \
+    git fetch $verbosity $progress --update-head-ok "$@" || exit 1
 
 curr_head=$(git rev-parse -q --verify HEAD)
 if test -n "$orig_head" && test "$curr_head" != "$orig_head"

-- 
1.7.0.2.204.g8940d.dirty

Alex Riesen· Mar 26, 2010, 23:12 UTC · re: Matthieu Moy · lore

Re: [PATCH v2] pull: mention "pull", not "fetch" in the error message.

Matthieu Moy, Fri, Mar 26, 2010 16:03:56 +0100:

> For newbies who've just been taught "git pull", the error message
> "Where do you want to fetch from today?" is indeed confusing. Change it
> to "Where do you want to pull from today?" in case fetch was called from
> pull.

...

Show 5 quoted lines

> @@ -842,8 +842,12 @@ static int fetch_one(struct remote *remote, int argc, const char **argv)
>  	int ref_nr = 0;
>  	int exit_code;
>  
> +	char *cmd = getenv("GIT_USER_COMMAND");

Isn't the variable name a little too generic? USER_COMMAND doesn't make it clear that its only purpose is to pass a string "pull" to an error message which is never even seen under normal circumstances.

Matthieu Moy· Mar 27, 2010, 08:52 UTC · re: Alex Riesen · lore

Re: [PATCH v2] pull: mention "pull", not "fetch" in the error message.

Alex Riesen <raa.lkml@gmail.com> writes:

Show 14 quoted lines

> Matthieu Moy, Fri, Mar 26, 2010 16:03:56 +0100:
>> For newbies who've just been taught "git pull", the error message
>> "Where do you want to fetch from today?" is indeed confusing. Change it
>> to "Where do you want to pull from today?" in case fetch was called from
>> pull.
> ...
>> @@ -842,8 +842,12 @@ static int fetch_one(struct remote *remote, int argc, const char **argv)
>>  	int ref_nr = 0;
>>  	int exit_code;
>>
>> +	char *cmd = getenv("GIT_USER_COMMAND");
>
> Isn't the variable name a little too generic? USER_COMMAND doesn't make it
> clear that its only purpose is to pass a string "pull"

It is generic, so that the same variable can be reused elsewhere if needed. When a command "git foo" calls internally "git bar", the instance of "git bar" thinks it's executing the command "bar", but the user thinks he just ran the command "foo". This variable lets git know that and adapt its error message accordingly.

> to an error message which is never even seen under normal
> circumstances.

For some definition of "never ever" and/or "normal circumstances" only, then:

/tmp/test$ git init Initialized empty Git repository in /tmp/test/.git/ /tmp/test$ git pull fatal: Where do you want to pull from today?

(the confused newbie scenario isn't imaginary, it just happened to a colleague of mine)

-- Matthieu Moy http://www-verimag.imag.fr/~moy/

Junio C Hamano· Mar 28, 2010, 17:14 UTC · re: Matthieu Moy · lore

Re: [PATCH v2] pull: mention "pull", not "fetch" in the error message.

Matthieu Moy <Matthieu.Moy@imag.fr> writes:

Show 28 quoted lines

> For newbies who've just been taught "git pull", the error message
> "Where do you want to fetch from today?" is indeed confusing. Change it
> to "Where do you want to pull from today?" in case fetch was called from
> pull.
>
> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
> ---
> Just fixed a typo in commit message since v1.
>
>  builtin/fetch.c |    6 +++++-
>  git-pull.sh     |    3 ++-
>  2 files changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/builtin/fetch.c b/builtin/fetch.c
> index 957be9f..f3246f5 100644
> --- a/builtin/fetch.c
> +++ b/builtin/fetch.c
> @@ -842,8 +842,12 @@ static int fetch_one(struct remote *remote, int argc, const char **argv)
>  	int ref_nr = 0;
>  	int exit_code;
>  
> +	char *cmd = getenv("GIT_USER_COMMAND");
> +	if (cmd == NULL || cmd[0] == '\0')
> +		cmd = "fetch";
> +
>  	if (!remote)
> -		die("Where do you want to fetch from today?");
> +		die("Where do you want to %s from today?", cmd);

Show 13 quoted lines

>  	transport = transport_get(remote, NULL);
>  	transport_set_verbosity(transport, verbosity, progress);
> diff --git a/git-pull.sh b/git-pull.sh
> index 1a4729f..abc233b 100755
> --- a/git-pull.sh
> +++ b/git-pull.sh
> @@ -216,7 +216,8 @@ test true = "$rebase" && {
>  	done
>  }
>  orig_head=$(git rev-parse -q --verify HEAD)
> -git fetch $verbosity $progress --update-head-ok "$@" || exit 1
> +GIT_USER_COMMAND=pull \
> +    git fetch $verbosity $progress --update-head-ok "$@" || exit 1

I personally don't think this is a good change. When somebody writes a wrapper around "git pull" to "make it easier", the name of the command that is given in the message will still be "pull", and you will have exactly the same issue again.

It was never the intent of the use of word "fetch" in the original message to give the name of one among many commands (i.e. the message was not about "fetch vs pull"). The word was to convey more abstract concept of getting new changes that are available on the other end.

An alternative that does not introduce layering violation of making the lower level porcelain aware of what the caller is doing might be to make it even more bland, "Where do you want to get changes from today?", or something.

Or we could even make it even more explicit by saying "Where do you want to go today?", which was what the original meant to mock ;-).