threads / patch / 17055

patchgit-commit: colored status when color.ui is set

Subject: [PATCH, resend] git-commit: colored status when color.ui is set

## tl;dr

4 messages between Jan 8, 2009 and Jan 9, 2009. Diffs are folded; open one to read it.

replies: 3people: 3as markdown or json

Markus Heidelberg· Jan 8, 2009, 18:53 UTC · lore

When using "git commit" and there was nothing to commit (the editor wasn't launched), the status output wasn't colored, even though color.ui was set. Only when setting color.status it worked.

Signed-off-by: Markus Heidelberg <markus.heidelberg@web.de>
---
 builtin-commit.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

Show changes to builtin-commit.c +3 −0

diff --git a/builtin-commit.c b/builtin-commit.c
index e88b78f..2d90f74 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -945,6 +945,9 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 
 	git_config(git_commit_config, NULL);
 
+	if (wt_status_use_color == -1)
+		wt_status_use_color = git_use_color_default;
+
 	argc = parse_and_validate_options(argc, argv, builtin_commit_usage, prefix);
 
 	index_file = prepare_index(argc, argv, prefix);

-- 
1.6.1.35.g0c23

Junio C Hamano· Jan 9, 2009, 09:00 UTC · re: Markus Heidelberg · lore

Re: [PATCH, resend] git-commit: colored status when color.ui is set

Markus Heidelberg <markus.heidelberg@web.de> writes:

Show 23 quoted lines

> When using "git commit" and there was nothing to commit (the editor
> wasn't launched), the status output wasn't colored, even though color.ui
> was set. Only when setting color.status it worked.
>
> Signed-off-by: Markus Heidelberg <markus.heidelberg@web.de>
> ---
>  builtin-commit.c |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/builtin-commit.c b/builtin-commit.c
> index e88b78f..2d90f74 100644
> --- a/builtin-commit.c
> +++ b/builtin-commit.c
> @@ -945,6 +945,9 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
>  
>  	git_config(git_commit_config, NULL);
>  
> +	if (wt_status_use_color == -1)
> +		wt_status_use_color = git_use_color_default;
> +
>  	argc = parse_and_validate_options(argc, argv, builtin_commit_usage, prefix);
>  
>  	index_file = prepare_index(argc, argv, prefix);

My first reaction was:

	When the editor does get launched, what would the new code do with
	your patch?  Would we see bunch of escape codes in the editor now?

But we do disable color explicitly when we generate contents to feed the editor in that case since bc5d248 (builtin-commit: do not color status output shown in the message template, 2007-11-18), so that fear is unfounded.

Thanks for a reminder, will queue.

Johannes Schindelin· Jan 9, 2009, 10:56 UTC · re: Junio C Hamano · lore

Re: [PATCH, resend] git-commit: colored status when color.ui is set

Hi,

On Fri, 9 Jan 2009, Junio C Hamano wrote:

Show 35 quoted lines

> Markus Heidelberg <markus.heidelberg@web.de> writes:
> 
> > When using "git commit" and there was nothing to commit (the editor
> > wasn't launched), the status output wasn't colored, even though color.ui
> > was set. Only when setting color.status it worked.
> >
> > Signed-off-by: Markus Heidelberg <markus.heidelberg@web.de>
> > ---
> >  builtin-commit.c |    3 +++
> >  1 files changed, 3 insertions(+), 0 deletions(-)
> >
> > diff --git a/builtin-commit.c b/builtin-commit.c
> > index e88b78f..2d90f74 100644
> > --- a/builtin-commit.c
> > +++ b/builtin-commit.c
> > @@ -945,6 +945,9 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
> >  
> >  	git_config(git_commit_config, NULL);
> >  
> > +	if (wt_status_use_color == -1)
> > +		wt_status_use_color = git_use_color_default;
> > +
> >  	argc = parse_and_validate_options(argc, argv, builtin_commit_usage, prefix);
> >  
> >  	index_file = prepare_index(argc, argv, prefix);
> 
> My first reaction was:
> 
> 	When the editor does get launched, what would the new code do with
> 	your patch?  Would we see bunch of escape codes in the editor now?
> 
> But we do disable color explicitly when we generate contents to feed the
> editor in that case since bc5d248 (builtin-commit: do not color status
> output shown in the message template, 2007-11-18), so that fear is
> unfounded.

I had the same reaction, so I would like to see this reasoning in the commit message.

Ciao, Dscho

Markus Heidelberg· Jan 9, 2009, 16:24 UTC · re: Johannes Schindelin · lore

Re: [PATCH, resend] git-commit: colored status when color.ui is set

Johannes Schindelin, 09.01.2009:

Show 13 quoted lines

> Hi,
> 
> On Fri, 9 Jan 2009, Junio C Hamano wrote:
> 
> > Markus Heidelberg <markus.heidelberg@web.de> writes:
> > > When using "git commit" and there was nothing to commit (the editor
> > > wasn't launched), the status output wasn't colored, even though color.ui
> > > was set. Only when setting color.status it worked.
> > 
> > My first reaction was:
> > 
> > 	When the editor does get launched, what would the new code do with
> > 	your patch?  Would we see bunch of escape codes in the editor now?

Of course I tested this case :)

Show 7 quoted lines

> > But we do disable color explicitly when we generate contents to feed the
> > editor in that case since bc5d248 (builtin-commit: do not color status
> > output shown in the message template, 2007-11-18), so that fear is
> > unfounded.
> 
> I had the same reaction, so I would like to see this reasoning in the 
> commit message.

wt_status_use_color could have already been set during git_config() without this patch, just not with color.ui, but color.status, so this is not really a new case. But I can understand the reaction and don't have ocjections against this explanation in the commit message.

Markus