Re: [PATCHv2 2/3] grep: Colorize filename, line number, and separator

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 7, 2010, 19:02 UTC
Message-ID: <7v1vfwrm51.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1267980767-12617-3-git-send-email-lodatom@gmail.com>

Mark Lodato <lodatom@gmail.com> writes:

Show 8 quoted lines

> +	if (color) {
>  		if (!value)
>  			return config_error_nonbool(var);
> -		color_parse(value, var, opt->color_match);
> -		return 0;
> +		color_parse(value, var, color);
> +		if (!strcmp(color, GIT_COLOR_RESET))
> +			color[0] = '\0';

I don't know this "optimization" is warranted. I can understand that you are trying to help the user to save a handful of useless bytes per line of output in the normal case, but doesn't "color.cmd.foo = normal" set the value to an empty string already if that is what the user wants?

The user may know about his contents better than this code does; perhaps the payload sometimes has funny ANSI sequence in it and the user may be trying to explicitly reset at the beginning of the line for the "plain" part of the output by saying "reset". Wouldn't this make it impossible to do so?