color (red) for error messages on all git commands

color (red) for error messages on all git commands

[Neal Kreitzinger]

Problem Scenario:  programmer doesn't pay attention to messages as he 
becomes more familiar with git and as a result doesn't notice error messages 
until its too late.  Mistakes take longer to remediate because the original 
error message was not resolved at the time it occurred.

Desired Solution:  configure git to display all error messages in color, 
e.g. red, so that the programmer does not overlook error messages.

Does anyone know how to configure this in git?


v/r,
Neal 

Re: color (red) for error messages on all git commands

[Sverre Rabbelier]

Heya,

On Wed, Mar 24, 2010 at 23:44, Neal Kreitzinger <neal@rsss.com> wrote:
> Problem Scenario:  programmer doesn't pay attention to messages as he
> becomes more familiar with git and as a result doesn't notice error messages
> until its too late.  Mistakes take longer to remediate because the original
> error message was not resolved at the time it occurred.
>
> Desired Solution:  configure git to display all error messages in color,
> e.g. red, so that the programmer does not overlook error messages.

That's actually something I run into as well, I am so used to git just
doing what I want that I don't notice the error messages when they
occur. If this is indeed desired (probably controlled by color.warning
and color.error?) it should be fairly easy to implement (see usage.c).

-- 
Cheers,

Sverre Rabbelier

Re: color (red) for error messages on all git commands

[Alex Riesen]

On Wed, Mar 24, 2010 at 23:58, Sverre Rabbelier <srabbelier@gmail.com> wrote:
> On Wed, Mar 24, 2010 at 23:44, Neal Kreitzinger <neal@rsss.com> wrote:
>> Desired Solution:  configure git to display all error messages in color,
>> e.g. red, so that the programmer does not overlook error messages.

It is considered a good behavior for a UNIX program only to display
something if there is an error. Git just tries to follow this tradition.

> That's actually something I run into as well, I am so used to git just
> doing what I want that I don't notice the error messages when they
> occur. If this is indeed desired (probably controlled by color.warning
> and color.error?) it should be fairly easy to implement (see usage.c).

That said, I also like the idea. Don't forget to check for tty in the patch.

Re: color (red) for error messages on all git commands

[Michael J Gruber]

Alex Riesen venit, vidit, dixit 25.03.2010 00:12:
> On Wed, Mar 24, 2010 at 23:58, Sverre Rabbelier <srabbelier@gmail.com> wrote:
>> On Wed, Mar 24, 2010 at 23:44, Neal Kreitzinger <neal@rsss.com> wrote:
>>> Desired Solution:  configure git to display all error messages in color,
>>> e.g. red, so that the programmer does not overlook error messages.
> 
> It is considered a good behavior for a UNIX program only to display
> something if there is an error. Git just tries to follow this tradition.

Oh well... Then I presume I'm not the only one who is annoyed by
"Initialized empty Git repository in ...", especially when you clone
locally and that is the only response you get! I'm embracing for the
response to my upcoming RFC...

Note that we also do a lot of progress messages (fetch from remotes,
committing, checking out, ...), and that is why distinguishing
warnings/error messages makes a lot of sense.

>> That's actually something I run into as well, I am so used to git just
>> doing what I want that I don't notice the error messages when they
>> occur. If this is indeed desired (probably controlled by color.warning
>> and color.error?) it should be fairly easy to implement (see usage.c).
> 
> That said, I also like the idea. Don't forget to check for tty in the patch.

Me too :)

Michael

[PATCH RFC/RFD] clone: quell the progress report from init

[Michael J Gruber]

Currently, cloning a local repository results in the progress report
"Initialized empty Git repository in ..." from init but no other output,
leaving the user to wonder what happened to the actual cloning.

Therefore, quell the progress report from init (unless -v is given) to
be in line with the many other commands which report nothing on success.

Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
---
This is RFC/RFD. An actual patch will also adjust several tests to
the new expected output. Also, one might want to change init's default
verbosity level accordingly.

The current output is really confusing for local cloning. There are
several alternative ways to resolve this:
- Make the local cloning process report something.
- Check whether the clone is remote or not and report init or don't.

Currently, we really have two philosophies:
- report only on error
- be chatty on success

I don't think that's a problem, but "simple" commands (e.g. branch, init,
add) should be in the former camp.

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

diff --git a/builtin/clone.c b/builtin/clone.c
index 05f8fb4..f98c5c5 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -461,7 +461,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 		die("could not create leading directories of '%s'", git_dir);
 	set_git_dir(make_absolute_path(git_dir));
 
-	init_db(option_template, (option_verbosity < 0) ? INIT_DB_QUIET : 0);
+	init_db(option_template, (option_verbosity <= 0) ? INIT_DB_QUIET : 0);
 
 	/*
 	 * At this point, the config exists, so we do not need the
-- 
1.7.0.3.448.g82eeb

Re: [PATCH RFC/RFD] clone: quell the progress report from init

[Alex Riesen]

On Thu, Mar 25, 2010 at 09:46, Michael J Gruber
<git@drmicha.warpmail.net> wrote:
> Currently, we really have two philosophies:
> - report only on error
> - be chatty on success
>
> I don't think that's a problem, but "simple" commands (e.g. branch, init,
> add) should be in the former camp.

Right. Everytime add scrolls out the current terminal buffer I wish I
had it sent to /dev/null.

>  builtin/clone.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)

Sometimes I think a statistics like "1 byte inserted" are more appropriate :)

> -       init_db(option_template, (option_verbosity < 0) ? INIT_DB_QUIET : 0);
> +       init_db(option_template, (option_verbosity <= 0) ? INIT_DB_QUIET : 0);

Re: [PATCH RFC/RFD] clone: quell the progress report from init

[Michael J Gruber]

Alex Riesen venit, vidit, dixit 25.03.2010 10:02:
> On Thu, Mar 25, 2010 at 09:46, Michael J Gruber
> <git@drmicha.warpmail.net> wrote:
>> Currently, we really have two philosophies:
>> - report only on error
>> - be chatty on success
>>
>> I don't think that's a problem, but "simple" commands (e.g. branch, init,
>> add) should be in the former camp.
> 
> Right. Everytime add scrolls out the current terminal buffer I wish I
> had it sent to /dev/null.
> 
>>  builtin/clone.c |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> Sometimes I think a statistics like "1 byte inserted" are more appropriate :)

Yeah, I was thinking this would be the/a minimal patch, but I screwed it
with the commit message :)

> 
>> -       init_db(option_template, (option_verbosity < 0) ? INIT_DB_QUIET : 0);
>> +       init_db(option_template, (option_verbosity <= 0) ? INIT_DB_QUIET : 0);

Michael

Re: [PATCH RFC/RFD] clone: quell the progress report from init

[Tilo Schwarz]

On Thu, 25 Mar 2010 09:46:16 +0100, Michael J Gruber  
<git@drmicha.warpmail.net> wrote:

> Currently, cloning a local repository results in the progress report
> "Initialized empty Git repository in ..." from init but no other output,
> leaving the user to wonder what happened to the actual cloning.

/me, (as just being an interested git user reading this list), likes this  
patch: As a Unix user I always wonder after a "git clone", why git tells  
me something about an "empty Git repository" (I think I saw the same  
message initializing submodules). I'd rather prefer no message, indicating  
the command completed with success.

Regards,

     Tilo

Re: [PATCH RFC/RFD] clone: quell the progress report from init

[Junio C Hamano]

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

> Currently, cloning a local repository results in the progress report
> "Initialized empty Git repository in ..." from init but no other output,
> leaving the user to wonder what happened to the actual cloning.

With your patch, local variant will become totally silent, but it may want
to give some feedback like real transports do; it is not entirely clear
what it should say, though.  The progress report given by the real
transports are to assure the user that we are not stuck, and the local one
is usually reasonably fast.

It's pointless to say "initialized empty" and then say "I am working hard
to clone".  As the command always creates a new directory and clones into
it, it makes sense to say only the latter, and your 1-byte patch may be a
good first step in that direction.

An alternative might be to squelch init_db() like your patch did, and then
replace it with a message of our own, perhaps like this:

 $ git clone -n /var/tmp/gomi victim-003
 Cloning into victim-003...
 done.
 $ git clone -n file:///var/tmp/gomi victim-004
 Cloning into victim-004...
 remote: Counting objects: ...
 Receiving objects: 100% (120/120), done.

Here I am assuming we would add "done." to the local codepath.

Personally I like the total silence your patch gives (i.e. no "Cloning
into" line in either case, and no "done." in local case) slightly better;
others may disagree and/or have better ideas.

> This is RFC/RFD. An actual patch will also adjust several tests to
> the new expected output.

And documentation, especially the tutorials.

"git init" as a top-level command may want to stay as-is; it is not like
you run "init" over and over again.  But we probably should make it an
independent issue from the "clone" one.

Thanks.

Re: [PATCH RFC/RFD] clone: quell the progress report from init

[Nicolas Pitre]

On Fri, 26 Mar 2010, Junio C Hamano wrote:

> An alternative might be to squelch init_db() like your patch did, and then
> replace it with a message of our own, perhaps like this:
> 
>  $ git clone -n /var/tmp/gomi victim-003
>  Cloning into victim-003...
>  done.
>  $ git clone -n file:///var/tmp/gomi victim-004
>  Cloning into victim-004...
>  remote: Counting objects: ...
>  Receiving objects: 100% (120/120), done.
> 
> Here I am assuming we would add "done." to the local codepath.
> 
> Personally I like the total silence your patch gives (i.e. no "Cloning
> into" line in either case, and no "done." in local case) slightly better;
> others may disagree and/or have better ideas.

Personally I like your suggestion above.  A clone is not something you 
perform repeatedly, and it is the first thing that random people told to 
use Git to grab a piece of code will do.  Better give them some comfort 
by telling them what is happening.


Nicolas

Re: [PATCH RFC/RFD] clone: quell the progress report from init

[Junio C Hamano]

Nicolas Pitre <nico@fluxnic.net> writes:

> On Fri, 26 Mar 2010, Junio C Hamano wrote:
>
>> An alternative might be to squelch init_db() like your patch did, and then
>> replace it with a message of our own, perhaps like this:
>> 
>>  $ git clone -n /var/tmp/gomi victim-003
>>  Cloning into victim-003...
>>  done.
>>  $ git clone -n file:///var/tmp/gomi victim-004
>>  Cloning into victim-004...
>>  remote: Counting objects: ...
>>  Receiving objects: 100% (120/120), done.
>> 
>> Here I am assuming we would add "done." to the local codepath.
> ...
> Personally I like your suggestion above.  A clone is not something you 
> perform repeatedly, and it is the first thing that random people told to 
> use Git to grab a piece of code will do.  Better give them some comfort 
> by telling them what is happening.

Ok, I can agree with that.  People who like silence can always say -q ;-)

Re: [PATCH RFC/RFD] clone: quell the progress report from init

[Junio C Hamano]

Nicolas Pitre <nico@fluxnic.net> writes:

> Personally I like your suggestion above.  A clone is not something you 
> perform repeatedly, and it is the first thing that random people told to 
> use Git to grab a piece of code will do.  Better give them some comfort 
> by telling them what is happening.

Here is what such a change may look like.  I'll leave adjusting
documentation (namely, tutorials) and perhaps tests as an exercise to the
readers ;-)


 builtin/clone.c  |    6 +++++-
 t/t5601-clone.sh |    2 +-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index 05f8fb4..0bedde4 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -302,6 +302,8 @@ static const struct ref *clone_local(const char *src_repo,
 	transport = transport_get(remote, src_repo);
 	ret = transport_get_remote_refs(transport);
 	transport_disconnect(transport);
+	if (0 <= option_verbosity)
+		printf("done.\n");
 	return ret;
 }
 
@@ -461,7 +463,9 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 		die("could not create leading directories of '%s'", git_dir);
 	set_git_dir(make_absolute_path(git_dir));
 
-	init_db(option_template, (option_verbosity < 0) ? INIT_DB_QUIET : 0);
+	if (0 <= option_verbosity)
+		printf("Cloning into %s...\n", get_git_dir());
+	init_db(option_template, INIT_DB_QUIET);
 
 	/*
 	 * At this point, the config exists, so we do not need the
diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
index 2147567..678cee5 100755
--- a/t/t5601-clone.sh
+++ b/t/t5601-clone.sh
@@ -34,7 +34,7 @@ test_expect_success 'clone with excess parameters (2)' '
 test_expect_success 'output from clone' '
 	rm -fr dst &&
 	git clone -n "file://$(pwd)/src" dst >output &&
-	test $(grep Initialized output | wc -l) = 1
+	test $(grep Clon output | wc -l) = 1
 '
 
 test_expect_success 'clone does not keep pack' '

Re: [PATCH RFC/RFD] clone: quell the progress report from init

[Michael J Gruber]

Junio C Hamano venit, vidit, dixit 30.03.2010 07:18:
> Nicolas Pitre <nico@fluxnic.net> writes:
> 
>> Personally I like your suggestion above.  A clone is not something you 
>> perform repeatedly, and it is the first thing that random people told to 
>> use Git to grab a piece of code will do.  Better give them some comfort 
>> by telling them what is happening.
> 
> Here is what such a change may look like.  I'll leave adjusting
> documentation (namely, tutorials) and perhaps tests as an exercise to the
> readers ;-)

OK, I'll take it since I kicked this off.

>  builtin/clone.c  |    6 +++++-
>  t/t5601-clone.sh |    2 +-
>  2 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/builtin/clone.c b/builtin/clone.c
> index 05f8fb4..0bedde4 100644
> --- a/builtin/clone.c
> +++ b/builtin/clone.c
> @@ -302,6 +302,8 @@ static const struct ref *clone_local(const char *src_repo,
>  	transport = transport_get(remote, src_repo);
>  	ret = transport_get_remote_refs(transport);
>  	transport_disconnect(transport);
> +	if (0 <= option_verbosity)
> +		printf("done.\n");
>  	return ret;
>  }
>  
> @@ -461,7 +463,9 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
>  		die("could not create leading directories of '%s'", git_dir);
>  	set_git_dir(make_absolute_path(git_dir));
>  
> -	init_db(option_template, (option_verbosity < 0) ? INIT_DB_QUIET : 0);
> +	if (0 <= option_verbosity)
> +		printf("Cloning into %s...\n", get_git_dir());
> +	init_db(option_template, INIT_DB_QUIET);

Don't we want init to report at least with -v?

>  
>  	/*
>  	 * At this point, the config exists, so we do not need the
> diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
> index 2147567..678cee5 100755
> --- a/t/t5601-clone.sh
> +++ b/t/t5601-clone.sh
> @@ -34,7 +34,7 @@ test_expect_success 'clone with excess parameters (2)' '
>  test_expect_success 'output from clone' '
>  	rm -fr dst &&
>  	git clone -n "file://$(pwd)/src" dst >output &&
> -	test $(grep Initialized output | wc -l) = 1
> +	test $(grep Clon output | wc -l) = 1
>  '
>  
>  test_expect_success 'clone does not keep pack' '

Michael

Re: [PATCH RFC/RFD] clone: quell the progress report from init

[Alex Riesen]

On Tue, Mar 30, 2010 at 07:18, Junio C Hamano <gitster@pobox.com> wrote:
> Nicolas Pitre <nico@fluxnic.net> writes:
>
>> Personally I like your suggestion above.  A clone is not something you
>> perform repeatedly, and it is the first thing that random people told to

Well, I have git clone in my top-20 used commands. I'm reasonably sure
it is used many times in a row too.

>> use Git to grab a piece of code will do.  Better give them some comfort
>> by telling them what is happening.
>
> Here is what such a change may look like.  I'll leave adjusting
> documentation (namely, tutorials) and perhaps tests as an exercise to the
> readers ;-)

There still is no way to make things quiet by default.

And at least for me the annoyance was when using it in command line,
interactively. So yes, it makes it possible to make clone quieter, but I
wont use it, because I have to either make an alias for git (and that
on every system I might come upon), or type in "-q" every time, at which
case I can also live with a little more output.

But even if it is of no immediate use to me personall, I still like the patch:
now I can make some scripts quieter (with less risk of removing something
interesting by piping everything to /dev/null).

[PATCH] clone: quell the progress report from init and report on clone

[Michael J Gruber]

From: Junio C Hamano <gitster@pobox.com>

Currently, a local glit clone reports only initializing an empty
git dir, which is potentially confusing.

Instead, report that cloning is in progress and when it is done
(unless -q) is given, and suppres the init report (unless -v
is given).

Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
---
I checked the documentation, there is really no place where the output
of git clone is documented - no documentation or test change is necesessary.

I made it so that -v will restore the init message. Other than that this is really
your path, Junio.

 builtin/clone.c  |    4 ++++
 t/t5601-clone.sh |    2 +-
 2 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index 05f8fb4..6e0584c 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -302,6 +302,8 @@ static const struct ref *clone_local(const char *src_repo,
 	transport = transport_get(remote, src_repo);
 	ret = transport_get_remote_refs(transport);
 	transport_disconnect(transport);
+	if (0 <= option_verbosity)
+		printf("done.\n");
 	return ret;
 }
 
@@ -462,6 +464,8 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 	set_git_dir(make_absolute_path(git_dir));
 
 	init_db(option_template, (option_verbosity < 0) ? INIT_DB_QUIET : 0);
+	if (0 <= option_verbosity)
+		printf("Cloning into %s...\n", get_git_dir());
 
 	/*
 	 * At this point, the config exists, so we do not need the
diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
index 2147567..678cee5 100755
--- a/t/t5601-clone.sh
+++ b/t/t5601-clone.sh
@@ -34,7 +34,7 @@ test_expect_success 'clone with excess parameters (2)' '
 test_expect_success 'output from clone' '
 	rm -fr dst &&
 	git clone -n "file://$(pwd)/src" dst >output &&
-	test $(grep Initialized output | wc -l) = 1
+	test $(grep Clon output | wc -l) = 1
 '
 
 test_expect_success 'clone does not keep pack' '
-- 
1.7.0.3.448.g82eeb

Re: [PATCH] clone: quell the progress report from init and report on clone

[Junio C Hamano]

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

> From: Junio C Hamano <gitster@pobox.com>
>
> Currently, a local glit clone reports only initializing an empty
> git dir, which is potentially confusing.

glit?

> Instead, report that cloning is in progress and when it is done
> (unless -q) is given, and suppres the init report (unless -v
> is given).
>
> Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
> ---
> I checked the documentation, there is really no place where the output
> of git clone is documented - no documentation or test change is necesessary.

Thanks.

> I made it so that -v will restore the init message.

I wonder how that would be useful, though.  The new message already says
which directory is being worked on.  I didn't see a point of repeating
that and made it silent on purpose.

Re: [PATCH] clone: quell the progress report from init and report on clone

[Michael J Gruber]

On Fri, 02 Apr 2010 00:28 -0700, "Junio C Hamano" <gitster@pobox.com>
wrote:
> Michael J Gruber <git@drmicha.warpmail.net> writes:
> 
> > From: Junio C Hamano <gitster@pobox.com>
> >
> > Currently, a local glit clone reports only initializing an empty
> > git dir, which is potentially confusing.
> 
> glit?

Sorry, this glitch clomes from the clone clommand of glit.
 
> > Instead, report that cloning is in progress and when it is done
> > (unless -q) is given, and suppres the init report (unless -v
> > is given).
> >
> > Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
> > ---
> > I checked the documentation, there is really no place where the output
> > of git clone is documented - no documentation or test change is necesessary.
> 
> Thanks.
> 
> > I made it so that -v will restore the init message.
> 
> I wonder how that would be useful, though.  The new message already says
> which directory is being worked on.  I didn't see a point of repeating
> that and made it silent on purpose.
 
Well, the idea is that '-v' should do at least something, and often (for
other commands) shows the individual steps of an action. The steps here
are init+clone.
Personally, I don't really care for either option.

Michael
P.S.: I'm sparsely online this week. Please don't mind if my responses
come in late.

[PATCH] clone: quell the progress report from init and report on clone

[Michael J Gruber]

From: Junio C Hamano <gitster@pobox.com>

Currently, a local git clone reports only initializing an empty
git dir, which is potentially confusing.

Instead, report that cloning is in progress and when it is done
(unless -q) is given, and suppress the init report.

Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
---
Basically this is your patch now, Junio, which is why it disappeared from my
radar. But I don't think you applied it, so I'm resubmitting.

v2 shuts up init unconditionally (even with -v), as you preferred.

 builtin/clone.c  |    6 +++++-
 t/t5601-clone.sh |    2 +-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index 05f8fb4..0bedde4 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -302,6 +302,8 @@ static const struct ref *clone_local(const char *src_repo,
 	transport = transport_get(remote, src_repo);
 	ret = transport_get_remote_refs(transport);
 	transport_disconnect(transport);
+	if (0 <= option_verbosity)
+		printf("done.\n");
 	return ret;
 }
 
@@ -461,7 +463,9 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 		die("could not create leading directories of '%s'", git_dir);
 	set_git_dir(make_absolute_path(git_dir));
 
-	init_db(option_template, (option_verbosity < 0) ? INIT_DB_QUIET : 0);
+	if (0 <= option_verbosity)
+		printf("Cloning into %s...\n", get_git_dir());
+	init_db(option_template, INIT_DB_QUIET);
 
 	/*
 	 * At this point, the config exists, so we do not need the
diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
index 2147567..678cee5 100755
--- a/t/t5601-clone.sh
+++ b/t/t5601-clone.sh
@@ -34,7 +34,7 @@ test_expect_success 'clone with excess parameters (2)' '
 test_expect_success 'output from clone' '
 	rm -fr dst &&
 	git clone -n "file://$(pwd)/src" dst >output &&
-	test $(grep Initialized output | wc -l) = 1
+	test $(grep Clon output | wc -l) = 1
 '
 
 test_expect_success 'clone does not keep pack' '
-- 
1.7.1.rc1.248.gcefbb